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

[Baptiste Coudurier]

Hi,

On 06/13/2011 02:56 AM, Joseph Artsimovich wrote:
> On 11/06/2011 01:05, Michael Niedermayer wrote:
>>> it might make sense to do the abs first then check against a threshold
>>> so the multiply&  shift is only done when the value isnt going to be
>>> 0 anyway. 0 is the most common value
>>> so this may be faster
>> also switching to 10 bit PIXFMT as baptiste suggested could be done
>>
>> But none of these things should cause the patch to be delayed for
>> months. the regression test failure must be fixed though
>> That is, if you prefer to work on the rest later iam in favor of
>> applying (as i planed) once the regression is fixed / explained
> The attached patch fixes the regression.  To be applied on top of the
> other patches.
> 
> 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.

> 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.

> 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.

[...]

-- 
Baptiste COUDURIER
Key fingerprint          8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                           http://www.ffmpeg.org