Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: resync if cluster parsing fails while seeking

2017-12-10 Thread Rodger Combs


> On Dec 10, 2017, at 08:08, Nicolas George  wrote:
> 
> Rodger Combs (2017-12-08):
>> ---
>> libavformat/matroskadec.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 496499b553..b51f67af00 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -3528,9 +3528,13 @@ static int matroska_read_seek(AVFormatContext *s, int 
>> stream_index,
>>   SEEK_SET);
>> matroska->current_id = 0;
>> while ((index = av_index_search_timestamp(st, timestamp, flags)) < 0 
>> || index == st->nb_index_entries - 1) {
>> +int ret;
>> +int64_t pos = avio_tell(matroska->ctx->pb);
>> matroska_clear_queue(matroska);
>> -if (matroska_parse_cluster(matroska) < 0)
>> -break;
>> +if ((ret = matroska_parse_cluster(matroska)) < 0) {
> 
>> +if ((ret == AVERROR_EOF) || matroska_resync(matroska, pos) 
>> < 0)
> 
> This is discarding the actual error code from matroska_resync().

This is the old read_seek API, so nothing looks at its actual return code 
(apart to check if it's >=0). In the new API, we'd want this, but I think 
moving to that is out of scope for this change.

> 
>> +break;
>> +}
>> }
>> }
>> 
> 
> Regards,
> 
> -- 
>  Nicolas George
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org 
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> 
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: resync if cluster parsing fails while seeking

2017-12-10 Thread Nicolas George
Rodger Combs (2017-12-08):
> ---
>  libavformat/matroskadec.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 496499b553..b51f67af00 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3528,9 +3528,13 @@ static int matroska_read_seek(AVFormatContext *s, int 
> stream_index,
>SEEK_SET);
>  matroska->current_id = 0;
>  while ((index = av_index_search_timestamp(st, timestamp, flags)) < 0 
> || index == st->nb_index_entries - 1) {
> +int ret;
> +int64_t pos = avio_tell(matroska->ctx->pb);
>  matroska_clear_queue(matroska);
> -if (matroska_parse_cluster(matroska) < 0)
> -break;
> +if ((ret = matroska_parse_cluster(matroska)) < 0) {

> +if ((ret == AVERROR_EOF) || matroska_resync(matroska, pos) < 
> 0)

This is discarding the actual error code from matroska_resync().

> +break;
> +}
>  }
>  }
>  

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: resync if cluster parsing fails while seeking

2017-12-10 Thread Carl Eugen Hoyos
2017-12-09 3:24 GMT+01:00 Rodger Combs :

> -if (matroska_parse_cluster(matroska) < 0)
> -break;
> +if ((ret = matroska_parse_cluster(matroska)) < 0) {
> +if ((ret == AVERROR_EOF) || matroska_resync(matroska, pos) < 
> 0)

If you believe the brackets improve readability, please keep them!

If you agree with me that they make reading the code more
difficult if anything, please remove them.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/3] lavf/matroskadec: resync if cluster parsing fails while seeking

2017-12-09 Thread Michael Niedermayer
On Fri, Dec 08, 2017 at 06:45:34AM -0600, Rodger Combs wrote:
> ---
>  libavformat/matroskadec.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

can you add a fate test for this commit or patchset ?

thx

[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel