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

Tim Nicholson nichot20 at yahoo.com
Thu Jan 24 13:52:23 CET 2013


On 24/01/13 10:44, Matthieu Bouron wrote:
> On Thu, Jan 24, 2013 at 10:27 AM, Tim Nicholson <nichot20 at yahoo.com> wrote:
>> 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:
> [...]
> 
>>From what i understand, 60Mbps and 90Mbps are not added twice to the array
> since a single value can be used for comparaison to choose the right cid to use.

True, I hadn't noticed that the current code didn't check that the
bitrate was valid for the requested framerate in order to decide the
cid, only that the bitrate was one of those allowed for the
framesize/interlace/bit depth.

> This is why i supposed that the 75Mbps refers to the 29.97 framerate.
> 
> The patch i prepared to associate bitrates to framerates will add the
> redundant values
> as follow:
> 
> bitrates = { 60, 60, 75, 120, 145 },
> framerates = { { 24000, 1001 }, { 25, 1 }, { 30000, 1001 }, { 50, 1 },
> { 60000 / 1001 } },
>
>[..]
>> 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?"
> 
> IMHO, the patch addresses "all" the inconsistencies i found in regards of
> the way the actual code works. Redudant value like 60Mbps or 90Mbps can
> be added in the same time as the framerates array.
> 
> I'll send the patch to the ml as soon as i can (likely to be tonight).
> 
Ahh, I did not know that there was a second part yet to come that
effectively sorted all the bits out in a consistent manner as I was
suggesting...

Does this mean that specifying the incorrect bitrate for the requested
framerate will no longer be possible, or at least flagged as incorrect?

Seems like my issues are nothing to do with your patch(es) but the
current implementation, which I only became aware of when looking at
your patch.


> Regards,
> Matthieu
> 
>>>
>>> Matthieu
>>>
>>> [...]



-- 
Tim


More information about the ffmpeg-devel mailing list