[MPlayer-dev-eng] [PATCH] fix aspect ratio bug for MPEG1 PAL 16/9

[Ivan Kalvachev]

2006/8/23, Giacomo Comes <comes at naic.edu>:
> I have found a small bug of libavcodec during the creation of mpeg1 video.
> Here is the description of the problem.
>
> The following command is a simple one for creating MPEG1 PAL video with 16/9
> aspect ratio:
>
> mencoder -of mpeg -mpegopts format=xvcd -ovc lavc -lavcopts vcodec=mpeg1video:vrc_buf_size=327:vrc_minrate=1152:vrc_maxrate=1152:keyint=15:vmax_b_frames=2:aspect=16/9:vbitrate=1152 -o test.mpg source.avi -ofps 25 -vf scale=352:288,harddup -oac lavc -lavcopts acodec=mp2:abitrate=224
>
> The MPEG1 standard says that the values for the 4 bit pel_aspect_ratio field are:
>
> pel  w/h
>  3  0.7031   16:9 625 lines
>  6  0.8437   16:9 525 lines
>  8  0.9375    4:3 625 lines
> 12  1.1250    4:3 525 lines
>
> libavcodec set the correct value for the aspect ratio in all cases except the first
> one. MPEG1 PAL 16/9 instead of having aspect 3 has aspect 2.
>
> The code that set the aspect ratio in the function mpeg1_encode_sequence_header
> in the file mpeg12.c is:
>
>     for(i=1; i<15; i++){
>         float error= aspect_ratio;
>         if(s->codec_id == CODEC_ID_MPEG1VIDEO || i <=1)
>             error-= 1.0/mpeg1_aspect[i];
>         else
>             error-= av_q2d(mpeg2_aspect[i])*s->height/s->width;
>         error= ABS(error);
>         if(error < best_aspect_error){
>             best_aspect_error= error;
>             s->aspect_ratio_info= i;
>         }
>     }
>
> The code itself is not wrong: for PAL 16/9, the aspect ratio that gives the smallest
> best_aspect_error is really 2 (2 -> 0.030235, 3 -> 0.032272), but the standard
> says that in this case the aspect has to be 3. Other tools, like mjpegtools,
> follow the standard and set 3 as aspect ratio for MPEG1 PAL 16/9.
>
> The attached patch check for the specific case (MPEG1 PAL 16/9) and set aspect 3,
> leaving all the other cases to the for loop.
>
> If you think the patch is correct and should be committed, please forward to the
> appropriate list.

This part of mplayer is imported from FFmpeg project and you have to
post it to ffmpeg-dev maillist.


I'm sure your patch won't be accepted. It always forces #3 when
aspect=16/9 is required despite that according to your reasoning #6 is
also 16/9 .
There is no code to detect PAL vs NTSC mode at all. Nor it takes image
size in calculations.

However I'm not completely sure that your problem is mpeg12.c encoder
fault.  The standard says that the encoder have to pick the closest
value and all decoders would take the aspect in consideration no
matter what it is.. This means that the PAL / NTSC values are just
examples.

In short the encoder should not try to second guess what the user want.

You had probably missed something. e.g.

now you have (352/288)/(16/9) = 0.6875
if you had      (360/288)/(16/9) = 0.703125

You see in the first case the closest match is #2 = 0.6735
(#2 delta=0.0140 vs.  #3 delta=0.0156)
in the second case it is nearly perfect match #3 = 0.7031
(delta=0.000025)


(I excuse in advance if I've done some stupid calculation mistake ;)