[FFmpeg-devel] [RFC]lavc/mjpegenc_common: Fix aspect ratio

[Carl Eugen Hoyos]

2018-12-10 9:29 GMT+01:00, Moritz Barsnick <barsnick at gmx.net>:
> On Mon, Dec 10, 2018 at 02:54:54 +0100, Carl Eugen Hoyos wrote:
>> I hope somebody can prove me wrong!
>
> Not sure, see below.
>
>> This would need a slightly more sophisticated update to the decoder.
>
> Because it is also presumed incorrect?
>
>>          put_bits(p, 16, 0x0102);
>>          put_bits(p,  8, 0);              /* units type: 0 - aspect ratio
>> */
>> -        put_bits(p, 16, sar.num);
>>          put_bits(p, 16, sar.den);
>> +        put_bits(p, 16, sar.num);
>>          put_bits(p, 8, 0); /* thumbnail width */
>
> If a 640x480 (640 x dimension, 480 y dimension) image has a SAR of 4/3,
> 4 (or the x dimension) is its numerator, 3 (or the y dimension) is its
> denominator.
>
> The spec (and Wikipedia) says:
>   units (1 byte) Units for the X and Y densities.
>     units = 0: no units, X and Y specify the pixel aspect ratio
>     units = 1: X and Y are dots per inch
>     units = 2: X and Y are dots per cm
>   Xdensity (2 bytes) Horizontal pixel density
>   Ydensity (2 bytes) Vertical pixel density
>
> So Xdensity and Ydensity should be (e.g.) 4 and 3 in this case, which
> are sar.num and sar.den (judging by the struct's field names),
> respectively.
>
> It seems to me that that is what the original implementation does.

The "density" (unless they intentionally reversed the meaning of the
word) is always the inverse of the aspect ratio.
Did they reverse the meaning in your opinion?
Does it make sense to argue that if the unit is "0", the meaning of the
density changes?

Carl Eugen