[FFmpeg-devel] [PATCH] CrystalHD decoder support v2

[Vladimir Pantelic]

On 12/29/2010 06:51 PM, Philip Langdale wrote:
> 
> On Wed, 29 Dec 2010 09:42:21 +0100, Vladimir Pantelic
> <vladoman at gmail.com> wrote:
>> Philip Langdale wrote:
>>
>>> +                /*
>>> +                 * Despite being notionally opaque, either
>>> libcrystalhd or
>>> +                 * the hardware itself will mangle pts values that
>>> are too
>>> +                 * small or too large. The docs claim it should be
>>> in uints
>>> +                 * of 100ns. Given that we're nominally dealing with
>>> a black
>>> +                 * box on both sides, any transform we do has no
>>> guarantee of
>>> +                 * avoiding mangling but, empirically, scalling as
>>> if the
>>> +                 * reorded_opaque value is in ms seems to work.
>>> +                 */
>>> +                uint64_t pts = avctx->reordered_opaque ==
>>> AV_NOPTS_VALUE ?
>>> +                               0 : avctx->reordered_opaque * 1000 *
>>> 100;
>>> +                av_log(priv->avctx, AV_LOG_VERBOSE, "input \"pts\":
>>> %lu\n",
>>> +                       avctx->reordered_opaque);
>>
>> this still look wrong to me, the value of "reordered_opaque" can be
>> *anything*
>> It is true that it is used to transport a timestamp by e.g. ffmpeg or
>> ffplay,
>> but you cannot assume that it will always be the case. The application
>> will
>> expect this value to be returned reordered but not modified.
> 
> I dug through the library code and for the 70015, it really looks like
> they scale
> down and up by 10000 (not 100000 as I used), so I don't have a choice
> but to try
> and defend against it. it seems that the timebase the hardware uses
> changed between
> the 70012 (100ns units) and the 70015 (1ms units) so they kept the
> library interface
> at 100ns units and then convert back and forth. There isn't any other
> place I can
> carry the value around with. :-/

yes, but still you have to try to keep the value intact. And you
cannot rely on reordered_opaque to be a timestamp, so I think your
1st approach to use pkt->pts was better. If you cannot attach
arbitrary data to a frame, you might keep a list/queue where you
keep track about which opaque value belongs to which timestamp.