[FFmpeg-devel] [PATCH] 8088flex TMV demuxer and decoder

[Daniel Verkamp]

On Thu, Apr 23, 2009 at 3:17 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Wed, Apr 22, 2009 at 12:37:14AM -0500, Daniel Verkamp wrote:
>> + ? ?if (AV_RL32(p->buf) == TMV_TAG)
>
> Could also use strncmp...
>

Surely strncmp would be both larger and slower (at least on LE)?  On x86-64:

original:

   text    data     bss     dec     hex filename
    901     144       0    1045     415 libavformat/tmv.o

0000000000000000 <tmv_probe>:
   0:   48 8b 57 08             mov    0x8(%rdi),%rdx
   4:   31 c0                   xor    %eax,%eax
   6:   81 3a 54 4d 41 56       cmpl   $0x56414d54,(%rdx)
   c:   ba 64 00 00 00          mov    $0x64,%edx
  11:   0f 44 c2                cmove  %edx,%eax
  14:   c3                      retq

strncmp:

   text    data     bss     dec     hex filename
    914     144       0    1058     422 libavformat/tmv.o

00000000000002a0 <tmv_probe>:
 2a0:   48 8b 77 08             mov    0x8(%rdi),%rsi
 2a4:   b9 04 00 00 00          mov    $0x4,%ecx
 2a9:   bf 00 00 00 00          mov    $0x0,%edi
 2ae:   f3 a6                   repz cmpsb %es:(%rdi),%ds:(%rsi)
 2b0:   0f 92 c2                setb   %dl
 2b3:   0f 97 c0                seta   %al
 2b6:   28 d0                   sub    %dl,%al
 2b8:   0f be c0                movsbl %al,%eax
 2bb:   83 f8 01                cmp    $0x1,%eax
 2be:   19 c0                   sbb    %eax,%eax
 2c0:   83 e0 64                and    $0x64,%eax
 2c3:   c3                      retq


>> + ? ?if (get_le32(pb) != TMV_TAG)
>> + ? ? ? ?return -1;
>
>> if (get_le32(pb) != AL_RL32("TMAV"))
> would be possible, too.
>

Ok, I suppose this is slightly clearer, but it increases code size, at
least on x86-64.

>> + ? ?ast->codec->sample_rate = get_le16(pb);
>
> Is this necessary? I see it in a lot of demuxers, when an why should
> that be set?
> I'd think just av_set_pts_info should be enough...
>

It seems to be necessary; if left out, the Stream #... output from
ffmpeg does not have the sample rate.

>> + ? ?if (url_feof(pb))
>> + ? ? ? ?return AVERROR_EOF;
>
> Seems pointless, av_get_packet should catch it.
>

Maybe it should, but it doesn't - without these lines, transcoding to
another format hangs forever on the last frame.

>> + ? ?pkt->pts ? ? ? ? ? ? ? ? ? ? = tmv->pts[tmv->stream_index];
>> + ? ?tmv->pts[tmv->stream_index] += tmv->pts_inc[tmv->stream_index];
>
> I think you should get the same result if you just remove these and let
> ffmpeg calculate the pts. [...]

Right, these are removed from the latest patches.

> [...] You should then get working seeking when you
> set AVFMT_GENERIC_INDEX in AVInputFormat and set PKT_FLAG_KEY correctly
> (I think all frames are keyframes for this format).

Right, all frames are keyframes.

I tried setting the flags as you described, but trying to seek in
ffplay just restarts the video from the beginning.  Do I need to write
a seek function for AVInputFormat as well?

Also, duration reported when seeking seems to be calculated
incorrectly - I don't set it anywhere, but the calculated duration is
longer than the actual video.  Is this calculated on just the pts (and
how)?  Otherwise, how is it known before the full file is read?

Thanks,
-- Daniel Verkamp