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

Matthieu Bouron matthieu.bouron at gmail.com
Thu Jan 24 14:02:18 CET 2013


On Thu, Jan 24, 2013 at 1:52 PM, Tim Nicholson <nichot20 at yahoo.com> wrote:
> 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?

Associate an invalid framerate would no longer be possible.

The upcoming patch will comme in two parts:
  - associate bitrates with framerates and add a bitrate check in the
ff_dnxhd_find_cid function,
  - print valid dnxhd profiles when codec parameters are invalid.

>
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list