On Fri, Oct 19, 2012 at 8:04 PM, Luca Barbato <lu_z...@gentoo.org> wrote:
> On 10/19/2012 06:42 PM, Reinhard Tartler wrote:
>> On Wed, Oct 17, 2012 at 11:46 PM, Reinhard Tartler <siret...@gmail.com>
>> wrote:
>>> On Wed, Oct 17, 2012 at 11:32 PM, Måns Rullgård <m...@mansr.com> wrote:
>>>> Reinhard Tartler <siret...@tauware.de> writes:
>>>>
>>>>> From: Michael Niedermayer <michae...@gmx.at>
>>>>>
>>>>> Fixes Ticket1718
>>>>>
>>>>> Signed-off-by: Michael Niedermayer <michae...@gmx.at>
>>>>> (cherry picked from commit 93b240f4a59348c07d3d7e4862227f6949c51e14)
>>>>>
>>>>> Signed-off-by: Reinhard Tartler <siret...@tauware.de>
>>>>> ---
>>>>> libavcodec/mpegaudio_parser.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
>>>>> index c904873..4166f3f 100644
>>>>> --- a/libavcodec/mpegaudio_parser.c
>>>>> +++ b/libavcodec/mpegaudio_parser.c
>>>>> @@ -54,6 +54,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
>>>>> int inc= FFMIN(buf_size - i, s->frame_size);
>>>>> i += inc;
>>>>> s->frame_size -= inc;
>>>>> + state = 0;
>>>>>
>>>>> if(!s->frame_size){
>>>>> next= i;
>
> Big picture:
>
> uint32_t state= pc->state;
> int i;
> int next= END_NOT_FOUND;
>
> for(i=0; i<buf_size; ){
> if(s->frame_size){
> int inc= FFMIN(buf_size - i, s->frame_size);
> i += inc;
> s->frame_size -= inc;
>
> if(!s->frame_size){
> next= i;
> break;
>
> break;
> }
> }else{
> while(i<buf_size){
> int ret, sr, channels, bit_rate, frame_size;
>
> state= (state<<8) + buf[i++];
>
> ...
>
> in libavcodec/parser.c
>
> s = av_mallocz(sizeof(AVCodecParserContext));
>
> So the state starts 0, in the file then it gets
>
> the value set and so on. Nothing random here at all.
>
> "state" is fed to the header parser, so basically the patch proposes to
> reset it when you have a frame so you start afresh, the patch doesn't
> seem wrong, not sure if setting it on the !s->frame_size branch is more
> correct since I hadn't read the whole code, surely not using the
> previous buffer tail sounds correct.
>
> Here if you want to have a better look on more samples.
>
> diff --git a/libavcodec/mpegaudio_parser.c b/libavcodec/mpegaudio_parser.c
> index c904873..e9eee1f 100644
> --- a/libavcodec/mpegaudio_parser.c
> +++ b/libavcodec/mpegaudio_parser.c
> @@ -49,13 +49,19 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
> int i;
> int next= END_NOT_FOUND;
>
> + av_log(NULL, AV_LOG_INFO, "state start %d\n", state);
> +
> for(i=0; i<buf_size; ){
> if(s->frame_size){
> int inc= FFMIN(buf_size - i, s->frame_size);
> i += inc;
> s->frame_size -= inc;
> + state = 0;
> +
> + av_log(NULL, AV_LOG_INFO, "state %d\n", state);
>
> if(!s->frame_size){
> + state = 0;
> next= i;
> break;
> }
> @@ -87,7 +93,7 @@ static int mpegaudio_parse(AVCodecParserContext *s1,
> }
> }
> }
> -
> + av_log(NULL, AV_LOG_INFO, "state end %d\n", state);
> pc->state= state;
> if (ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
> *poutbuf = NULL;
I believe the av_log could also b made AV_LOG_DEBUG. So, how about
taking Luca's patch with a commit message along the lines:
mpegaudio_parser: reset parsing context state on new frames
?
--
regards,
Reinhard
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
