[FFmpeg-devel] [PATCH] dv: add timecode to metadata

Reimar Döffinger Reimar.Doeffinger at gmx.de
Fri Dec 23 14:26:50 CET 2011


On Fri, Dec 23, 2011 at 01:07:00AM +0100, Matthieu Bouron wrote:
> +    ff          = tc_pack[1]       & 0xf;
> +    ff         += (tc_pack[1] >> 4 & 0x3) * 10;
> +    drop_frame  = tc_pack[1]  >> 6 & 0x1;
> +    ss          = tc_pack[2]       & 0xf;
> +    ss         += (tc_pack[2] >> 4 & 0x7) * 10;
> +    mm          = tc_pack[3]       & 0xf;
> +    mm         += (tc_pack[3] >> 4 & 0x7) * 10;
> +    hh          = tc_pack[4]       & 0xf;
> +    hh         += (tc_pack[4] >> 4 & 0x3) * 10;

static int bcd2int(uint8_t bcd) {
    int low  = bcd & 0xf;
    int high = bcd >> 4;
    if (low > 9 || high > 9)
        return -1;
    return low + 10*high;
}

ff = bcd2int(tc_pack[1] & 0x3f);
ss = bcd2int(tc_pack[2] & 0x7f);
mm = bcd2int(tc_pack[3] & 0x7f);
hh = bcd2int(tc_pack[4] & 0x3f);

decide on your own if you want more error handling, the -1 is obvious
enough on its own in principle.
Of course you could just skip the whole conversion to int step and do
directly
tc[0] = '0' + (tc_pack[1] >> 4);
tc[1] = '0' + (tc_pack[1] & 0xf);
tc[2] = ':'
...
which in some ways is nicer/more obvious and the result of invalid
values > 9 probably has the same 'strange' effect as it will on a lot
of hardware.
Of course whether such "unvalidated" value in timecode metadata is good
or not is up to discussion, and it's not really possible to validate
against time codes with second values like 79.
I'd still be kind of in favour since it most directly exports the data
that is actually in the file, but some people dislike that.


> +    ret = avio_read(s->pb, partial_frame, partial_frame_size);
> +    if (ret < partial_frame_size)
> +        goto finish;
[...]
> +finish:
> +    av_free(partial_frame);
> +    avio_seek(s->pb, pos, SEEK_SET);
> +    return ret;

That is not quite right I think, you should not return >= 0 on error.
So for avio_read returning < 0 you should return that value,
however for avio_read returning >= 0 and < partial_frame_size
you should return -1 or AVERROR_EOF or AVERROR(EIO) or such.


More information about the ffmpeg-devel mailing list