[FFmpeg-devel] [PATCH] avformat/matroskadec: retain error codes in matroska_resync() and matroska_read_packet()

[Sophia Wang]

On Fri, Sep 23, 2016 at 1:40 AM, Benoit Fouet <benoit.fouet at free.fr> wrote:

> Hi,
>
>
> On 22/09/2016 23:03, Sophia Wang wrote:
>
>> Signed-off-by: Sophia Wang <skw at google.com>
>> ---
>>   libavformat/matroskadec.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 77b8a5d..936690d 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -738,13 +738,16 @@ static int matroska_read_close(AVFormatContext *s);
>>   static int matroska_resync(MatroskaDemuxContext *matroska, int64_t
>> last_pos)
>>   {
>>       AVIOContext *pb = matroska->ctx->pb;
>> +    int64_t ret;
>>       uint32_t id;
>>       matroska->current_id = 0;
>>       matroska->num_levels = 0;
>>         /* seek to next position to resync from */
>> -    if (avio_seek(pb, last_pos + 1, SEEK_SET) < 0)
>> -        goto eof;
>> +    if ((ret = avio_seek(pb, last_pos + 1, SEEK_SET)) < 0) {
>> +        matroska->done = 1;
>> +        return ret;
>>
>
> doesn't this generate a warning, returning an int64 from a function
> supposed to return an int?


avio_seek returns an int64, so at some point an int64 is going to be
converted to an int. I don't believe it will make a difference where,
especially since the values that will actually be returned (error codes)
should fit in an int.


>
> +    }
>>         id = avio_rb32(pb);
>>   @@ -760,7 +763,6 @@ static int matroska_resync(MatroskaDemuxContext
>> *matroska, int64_t last_pos)
>>           id = (id << 8) | avio_r8(pb);
>>       }
>>   -eof:
>>       matroska->done = 1;
>>       return AVERROR_EOF;
>>   }
>> @@ -3317,13 +3319,14 @@ static int matroska_parse_cluster(MatroskaDemuxContext
>> *matroska)
>>   static int matroska_read_packet(AVFormatContext *s, AVPacket *pkt)
>>   {
>>       MatroskaDemuxContext *matroska = s->priv_data;
>> +    int ret = 0;
>>         while (matroska_deliver_packet(matroska, pkt)) {
>>           int64_t pos = avio_tell(matroska->ctx->pb);
>>           if (matroska->done)
>> -            return AVERROR_EOF;
>> +            return (ret < 0) ? ret : AVERROR_EOF;
>>           if (matroska_parse_cluster(matroska) < 0)
>> -            matroska_resync(matroska, pos);
>> +            ret = matroska_resync(matroska, pos);
>>       }
>>         return 0;
>>
>
> You might want to return ret instead of 0 here.


Done.


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