[FFmpeg-devel] [PATCHv2] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback

[Ganesh Ajjanagadde]

On Tue, Jan 5, 2016 at 5:29 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Mon,  4 Jan 2016 17:50:01 -0800
> Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>
>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
>> checked here and a diagnostic is logged.
>>
>> All usage of ffio_ensure_seekback in the codebase now has the return value checked.
>>
>> Reviewed-by: wm4 <nfxjfg at googlemail.com>
>> Reviewed-by: Ronald S. Bultje <rsbultje at gmail.com>
>> Reviewed-by: Michael Niedermayer <michael at niedermayer.cc>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavformat/mp3dec.c | 13 +++++++++++--
>>  libavformat/rmdec.c  |  3 ++-
>>  2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>> index c76b21e..57ebcc8 100644
>> --- a/libavformat/mp3dec.c
>> +++ b/libavformat/mp3dec.c
>> @@ -372,11 +372,19 @@ static int mp3_read_header(AVFormatContext *s)
>>          uint32_t header, header2;
>>          int frame_size;
>>          if (!(i&1023))
>> -            ffio_ensure_seekback(s->pb, i + 1024 + 4);
>> +            if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0) {
>> +                av_log(s, AV_LOG_WARNING,
>> +                       "initial junk detection and skipping impossible due to: %s\n", av_err2str(ret));
>> +                goto skip_fail;
>> +            }
>>          frame_size = check(s->pb, off + i, &header);
>>          if (frame_size > 0) {
>>              avio_seek(s->pb, off, SEEK_SET);
>> -            ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
>> +            if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4)) < 0) {
>> +                av_log(s, AV_LOG_WARNING,
>> +                       "initial junk detection and skipping impossible due to: %s\n", av_err2str(ret));
>> +                goto skip_fail;
>> +            }
>>              if (check(s->pb, off + i + frame_size, &header2) >= 0 &&
>>                  (header & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
>>              {
>> @@ -387,6 +395,7 @@ static int mp3_read_header(AVFormatContext *s)
>>          }
>>          avio_seek(s->pb, off, SEEK_SET);
>>      }
>> +skip_fail:
>>
>>      // the seek index is relative to the end of the xing vbr headers
>>      for (i = 0; i < st->nb_index_entries; i++)
>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>> index 4e46a3d..470e227 100644
>> --- a/libavformat/rmdec.c
>> +++ b/libavformat/rmdec.c
>> @@ -618,7 +618,8 @@ static int rm_read_header(AVFormatContext *s)
>>              size = avio_rb32(pb);
>>              codec_pos = avio_tell(pb);
>>
>> -            ffio_ensure_seekback(pb, 4);
>> +            if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>> +                av_log(s, AV_LOG_WARNING, "seeking back impossible due to: %s\n", av_err2str(ret));
>>              v = avio_rb32(pb);
>>              if (v == MKBETAG('M', 'L', 'T', 'I')) {
>>                  ret = rm_read_multi(s, s->pb, st, mime);
>
> I maintain that this should not be done, because it makes the code
> verbose for no reason.

Michael has pointed out that when seekback fails, one should really
break out of the junk detection loop in mp3dec. Your idea fails to
achieve it.

> Or if you insist, do it in the ffio_ function
> itself.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel