[FFmpeg-devel] [PATCH] Add writing of vorbis comments to flac files

[Justin Ruggles]

James Darnley wrote:

> Another patch for consideration.
> 
> On 3 February 2010 18:55, James Darnley <james.darnley at gmail.com> wrote:
>>> -ret: 0         st: 0 flags:1 dts: NOPTS    pts: NOPTS    pos:     42
>>> size:  1024
>>> +ret: 0         st: 0 flags:1 dts: NOPTS    pts: NOPTS    pos:   8256
>>> size:  1024
> 
> Okay, this seems to be the position of the first frame in the file.
> After looking at some other files, this value always pointed to the
> first frame.
> 
>> Does the AVMetadata system automatically use the following change?
>>> r21587 | pross | 2010-02-01 12:39:10 +0100 (Mon, 01 Feb 2010) | 3 lines
>>> Add a list of generic tags and change demuxers to follow it.
>>> Patch by Anton Khirnov, wyskas at gmail dot com
> 
> I tried using various keys but I cannot see that this does much.  So I
> added the two removed entries in ff_vorbiscomment_metadata_conv namely
> date-year and artist-author.

What specifically did you try to do that did not work?  You should not
need to have year and author converted since the generic tags should
already be date and artist.

> +
> +int flac_write_block_comment(ByteIOContext *pb, AVMetadata *m,
> +                             int last_block, int bitexact)
> +{
> +    const char *vendor = bitexact ? "ffmpeg" : LIBAVFORMAT_IDENT;
> +    unsigned int len;
> +    uint8_t *p, *p0;
> +
> +    len = ff_vorbiscomment_length(m, vendor);
> +    p0 = av_malloc(len+4);
> +    if (!p0)
> +        return AVERROR(ENOMEM);
> +    p = p0;
> +
> +    bytestream_put_byte(&p, last_block ? 0x84 : 0x04);
> +    bytestream_put_be24(&p, len);
> +    ff_vorbiscomment_write(&p, m, vendor);


If the bitexact flag is set, you should pass NULL instead of m so that
tags are not written.

> [...]
> +    /* The command line flac encoder defaults to placing a seekpoint
> +     * every 10s.  So one might add padding to allow that later
> +     * but there seems to be no simple way to get the duration here.
> +     * So let's try the flac default of 8192 bytes */
> +    flac_write_block_padding(s->pb, 8192, 1);


I would recommend adding the writing of a padding block in a separate
commit.  It is unrelated to writing of VorbisComment tags, and it is the
cause of the change to the regression tests.

-Justin