Philip de Nier <[email protected]> added the comment:
Hi,
Thanks for the advice, I'll split the patch and remove cosmetic changes.
>> @@ -801,31 +812,54 @@
>> + st->codec->sample_aspect_ratio =
(AVRational){40, 33};
>> + }
> I believe these sample aspect ratios are wrong for HD and wrong for
> SD without the clean aperture specified.
>
> FFmpeg is digital only so I recommend using the digital sample aspect
> ratio (DAR/PAR == width/height) since the clean aperture cannot
> really be specified currently. Patch welcome :)
Ok, I'll need to get some advice on what to do here. My initial thoughts are
that a reasonable assumption can be made on the sample aspect ratio given
sufficient checks on the type of source material. E.g. given the essence
container label (which specifies 625 line, 50i, 4:2:2 13.5MHz), 720x576
dimensions, the sample aspect ratio is x/y for 4:3 or k/l for 16:9.
> Can we really have depth == 10 and codec_id != V210 ? The V210
> bitstream is really specific regarding the packing of samples.
You can't have codec_id == V210 and depth != 10, but you can have depth == 10
and codec_id != V210 (S377-2009 lists a planar 10-bit format).
My thinking was that some older files might have specified depth == 10 but
haven't included the v210 label which was introduced into the latest revision of
the spec. This led me to allow depth == 10 and assume v210 as a reasonable best
guess. I could just stick to our MXF files and only support MXF files that have
the v210 label?
> Well <= 32 is more correct.
> If > 32 we need CODEC_ID_PCM_S64LE I guess :/
I could've started the ifs with a "if (descriptor->bits_per_sample > 32) then
show warning", but thought that was just being pedantic :-)
> Was this rule made applicable for all essence containers ?
> I know the check is not ideal in the first place :/
I've never come across something explicitly stating this rule. I guess this is
just some informal rule editors have followed when creating the labels. Not sure
whether there is something else that can definitely tell you what wrapping
method is used.
Philip
p.s. apologies for the rubbish formatting. It should be better after adding
myself to the nosy list.
----------
nosy: +philipn
_____________________________________________________
FFmpeg issue tracker <[email protected]>
<https://roundup.ffmpeg.org/roundup/ffmpeg/issue1470>
_____________________________________________________
