[FFmpeg-devel] ffmpeg: remove dead code out of the video filter loop

It makes no sense for this code to be inside the video filter loop, and
the frame sample aspect ratio is even set unconditionally a few lines
above.
---
 ffmpeg.c | 5 -----
 1 file changed, 5 deletions(-)

On Tue, Nov 01, 2016 at 10:53:40AM +0100, Clément Bœsch wrote:
> It makes no sense for this code to be inside the video filter loop, and
> the frame sample aspect ratio is even set unconditionally a few lines
> above.
> ---
>  ffmpeg.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 3b91710..7e366f2 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2180,7 +2180,6 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
>      int i, ret = 0, err = 0, resample_changed;
>      int64_t best_effort_timestamp;
>      int64_t dts = AV_NOPTS_VALUE;
> -    AVRational *frame_sample_aspect;
>      AVPacket avpkt;
>  
>      // With fate-indeo3-2, we're getting 0-sized packets before EOF for some
> @@ -2316,11 +2315,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
>          }
>      }
>  
> -    frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame, "sample_aspect_ratio");
>      for (i = 0; i < ist->nb_filters; i++) {
> -        if (!frame_sample_aspect->num)
> -            *frame_sample_aspect = ist->st->sample_aspect_ratio;
> -
>          if (i < ist->nb_filters - 1) {
>              f = ist->filter_frame;
>              err = av_frame_ref(f, decoded_frame);

I'll push this soon

On 17.11.2016 23:12, Clément Bœsch wrote:
> On Tue, Nov 01, 2016 at 10:53:40AM +0100, Clément Bœsch wrote:
>> It makes no sense for this code to be inside the video filter loop, and
>> the frame sample aspect ratio is even set unconditionally a few lines
>> above.
>> ---
>>  ffmpeg.c | 5 -----
>>  1 file changed, 5 deletions(-)
>>
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index 3b91710..7e366f2 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -2180,7 +2180,6 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
>>      int i, ret = 0, err = 0, resample_changed;
>>      int64_t best_effort_timestamp;
>>      int64_t dts = AV_NOPTS_VALUE;
>> -    AVRational *frame_sample_aspect;
>>      AVPacket avpkt;
>>  
>>      // With fate-indeo3-2, we're getting 0-sized packets before EOF for some
>> @@ -2316,11 +2315,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
>>          }
>>      }
>>  
>> -    frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame, "sample_aspect_ratio");
>>      for (i = 0; i < ist->nb_filters; i++) {
>> -        if (!frame_sample_aspect->num)
>> -            *frame_sample_aspect = ist->st->sample_aspect_ratio;
>> -
>>          if (i < ist->nb_filters - 1) {
>>              f = ist->filter_frame;
>>              err = av_frame_ref(f, decoded_frame);
> 
> I'll push this soon

Looking at git history reveals:
 * commit 4778783 (2012-04-17) added this
 * commit 695ec04 (2012-04-09) added the unconditional code above the loop
 * commit 6bfb304 (2012-04-09) is the merge commit for commit 695ec04

So this might have been a merge error. Removing this seems correct.

Best regards,
Andreas

On Fri, Nov 18, 2016 at 12:32:49AM +0100, Andreas Cadhalpun wrote:
> On 17.11.2016 23:12, Clément Bœsch wrote:
> > On Tue, Nov 01, 2016 at 10:53:40AM +0100, Clément Bœsch wrote:
> >> It makes no sense for this code to be inside the video filter loop, and
> >> the frame sample aspect ratio is even set unconditionally a few lines
> >> above.
> >> ---
> >>  ffmpeg.c | 5 -----
> >>  1 file changed, 5 deletions(-)
> >>
> >> diff --git a/ffmpeg.c b/ffmpeg.c
> >> index 3b91710..7e366f2 100644
> >> --- a/ffmpeg.c
> >> +++ b/ffmpeg.c
> >> @@ -2180,7 +2180,6 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
> >>      int i, ret = 0, err = 0, resample_changed;
> >>      int64_t best_effort_timestamp;
> >>      int64_t dts = AV_NOPTS_VALUE;
> >> -    AVRational *frame_sample_aspect;
> >>      AVPacket avpkt;
> >>  
> >>      // With fate-indeo3-2, we're getting 0-sized packets before EOF for some
> >> @@ -2316,11 +2315,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int eo
> >>          }
> >>      }
> >>  
> >> -    frame_sample_aspect= av_opt_ptr(avcodec_get_frame_class(), decoded_frame, "sample_aspect_ratio");
> >>      for (i = 0; i < ist->nb_filters; i++) {
> >> -        if (!frame_sample_aspect->num)
> >> -            *frame_sample_aspect = ist->st->sample_aspect_ratio;
> >> -
> >>          if (i < ist->nb_filters - 1) {
> >>              f = ist->filter_frame;
> >>              err = av_frame_ref(f, decoded_frame);
> > 
> > I'll push this soon
> 
> Looking at git history reveals:
>  * commit 4778783 (2012-04-17) added this
>  * commit 695ec04 (2012-04-09) added the unconditional code above the loop
>  * commit 6bfb304 (2012-04-09) is the merge commit for commit 695ec04
> 
> So this might have been a merge error. Removing this seems correct.
> 

Thanks, applied