[FFmpeg-devel] [PATCH] DNxHD 10-bit support v5

[Joseph Artsimovich]

Would someone look at my latest patch please?


On 14/06/2011 09:19, Joseph Artsimovich wrote:
> On 13/06/2011 22:40, Baptiste Coudurier wrote:
>>> As for the 10 bit vs 16 bit formats, let me give you my perspective.
>>> If I got that correctly, the comment in the header suggests storing
>>> [0..1023] samples in YUV422P16.  I am not a fan of that idea.  Is there
>>> currently even a way to tell libswscale how many bits are really being
>>> used?  Also keep in mind that some applications (ours included) want to
>>> do their own colour conversion, and introducing this idea of "storage
>>> bits != logical bits" might require architectural changes.
>>> Another option is to use 10-bit planar formats.  That sounds good in
>>> theory, but in practice it pushes the complexity out of ffmpeg, to
>>> libraries like libquicktime that uses ffmpeg internally, and also to
>>> applications that want to do their own colour conversion.
>>> The third option, which my patches use, is to scale 10 bit samples to
>>> the full 16bit range.  This approach is my favorite and is the least
>>> painful overall.
>> Well, I understand, but using YUV422P10 is just faster, simpler and more
>> efficient. I'm against the code uselessly using YUV422P16.
>> Also converting from P16 to P10 for v210 output or vice versa would be
>> just waste of cpu.
> Faster, more efficient - maybe, but simpler?  If I go that route, I 
> will be adding support for converting to / from this format both to 
> libquicktime (we use that instead of libavformat), and to our own 
> application as well.  Besides, I am not so sure additional operations 
> on bits will be slower than rescaling.
>
>>> 2. The "full range dnxhd" turned out to be more than full range.  I 
>>> call
>>> it "overstretched range".  That is, U and V values of 0 and 255 don't
>>> correspond to -0.5 and 0.5, instead they correspond to something like
>>> -0.488 and 0.488.
>> It seems weird to me that DNxHD and/or AVID would use a range that is
>> not referenced in SMPTE.
>> You seem to say the range is shorter ? It should be 4 - 1019 AFAIK.
> It's not shorter.  It's full range for Y and "wider than full" range 
> for U and V, so extreme U and V values are actually not representable 
> in this format.  I think the idea was: "they are not representable in 
> RGB either, so why waste bits on them".  From this point of view the 
> "Rgb Levels" name makes sense, except that what Avid calls "Rgb 
> Levels" is actually normal video range and what it calls "709 Levels" 
> is this weird format.
>
>>> respect_user_specified_idct_algo.patch
>>>
>>>
>>> diff --git a/libavcodec/dnxhddec.c b/libavcodec/dnxhddec.c
>>> index d995222..a06b6d3 100644
>>> --- a/libavcodec/dnxhddec.c
>>> +++ b/libavcodec/dnxhddec.c
>>> @@ -48,6 +48,7 @@ typedef struct DNXHDContext {
>>>       ScanTable scantable;
>>>       const CIDEntry *cid_table;
>>>       int initialized_for_bits; // 8, 10 or 0 if not initialized at 
>>> all.
>>> +    int dnx8bit_idct_algo;
>> Named it 'idct_algo' and merge this patch into the previous one
>> otherwise there is gonna be useless break in the tree.
>>
> Actually it's named dnx8bit_idct_algo on purpose, as the 10-bit code 
> never looks at it and never modifies it.
> Well, no big deal anyway.  I merged this patch into the previous one, 
> and made two versions of it, one with my naming, another with yours.  
> Pick any.
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-- 
Joseph Artsimovich
Software Developer
MirriAd Ltd