[FFmpeg-devel] [PATCH] lavc/dnxhddata: fix bitrates for cid 1251 and 1252 in cid table

[Tim Nicholson]

On 22/01/13 15:31, Matthieu Bouron wrote:
> On Tue, Jan 22, 2013 at 4:04 PM, Tim Nicholson <nichot20 at yahoo.com> wrote:
>> On 22/01/13 13:34, Matthieu Bouron wrote:
>>> On Tue, Jan 22, 2013 at 2:18 PM, Tim Nicholson <nichot20 at yahoo.com> wrote:
>>>> On 21/01/13 20:14, Matthieu Bouron wrote:
>>>>> ---
>>>>>
>>>>> Hi there,
>>>>>
>>>>> According to SMPTE S2019-1, bitrates should be:
>>>>>   - 180Mbps for cid 1251 at 50fps,
>>>>>   - 120Mbps for cid 1252 at 50fps.
>>>>
>>>> Looking at Annex F in my SMPTE S2019-1-2008 suggests 25fps figures are
>>>> wrong too..
>>>>
>>>> cid 1251 is { 90, 80, 180, 220 }
>>>> cid 1252 is { 60, 60, 120, 145 }
>>>>
>>>
>>> The 75Mbps value "may" refer to the 29.97fps profile (which is
>>> declared here: http://www.avid.com/static/resources/FR/documents/dnxhd.pdf
>>> but not in the S2019). The single 60Mbps value refers here to the
>>> 23.976fps and 25fps profiles (this is how i understand it).
>>
>>
>> So the original values are based on Avid's spec (although the 75 should
>> be 72), and you want to change some of them to conform to SMPTE VC3 spec?
>>
>> It is unhelpful that the values sources are not documented, since you
>> were quoting SMPTE I assumed the values that I suggested also needed a
>> tweak were for 25fps since 29.97fps does not exist in the cid's under
>> discussion...
>>
>> What is your thinking behind having values that appear to be drawn from
>> two different specification sources? Is this wise?
> 
> I do not know where the actual values came from, maybe Baptiste can
> comment on this.
> The only document i actually trust is the SMPTE S2019-1 and the avid
> document i linked was found by random googling when i was trying to
> look for a 75Mbps 720p profile.
> 
> I tried to correct thoses values to be more consitent with the SMPTE
> spec since the smpte bitrate (180) is used for the cid 1250 50fps but
> not for the cid 1251 50fps (175 instead of 180).
> 

I have no problem with that. I think my greater concern is that there
are also other figures in cid's 1251 & 1252 which are either wrong for
25fps, or relate to the rate for a frame rate that is not listed in the
VC3 Annex, while one that is listed is not provided for. This is
inconsistent with the approach for other cid's in the list, see below.


> IMHO, mixing the missing dnx profiles with the vc3 ones sounds ok
> untill those dnx profiles are effectively supported by ffmpeg.

I think that is probably OK provided it is clear where the figures come
from, and which values in the array relate to what.

Looking at other cid's, for example 1237, the SMPTE Annexe only lists:-
23.976, 25, 50 and 59.94 data rates whereas Avid list:-
23.976/24, 25, 29.97, 50 59.94/60

The ffmpeg CIDEntry is:-

{ 115, 120, 145, 240, 290 }

which is basically the SMPTE values with the extra 29.97 figure (145)
added in in between the 25 and 50 values. This seems to me to be a
sensible way of "mixing the missing dnx profiles".

However this suggests that you supposition that 'The 75Mbps value "may"
refer to the 29.97fps profile' is not correct. Following the above
example I would have expected:-

{ 90, 90, 110, 180, 220 }
and
{ 60, 60, 75, 120, 145 }

I note however that cids 1241-43 only have 2 entries in the array as per
both the Annex and Avid, however the fact that the array is variable
length makes it impossible to relate the array index absolutely to a
frame rate, and confirm or deny your supposition.

According to the header file this array is a "Helpher to choose
variants, rounded to nearest 5Mb/s" so how is it actually used?
Would not the 5 entry array be more appropriate?

In summary, what I am really asking is,"does your patch address all the
errors/inconsistencies in this area, and would it not be better to do it
in one hit rather than dribs and drabs?"
> 
> Matthieu
> 
> [...]


-- 
Tim