[FFmpeg-devel] [PATCH 0/4] User controllable padding

[James Almer]

On 30/12/13 3:38 PM, James Darnley wrote:
> Patch 1: Adds a new field to AVFormatContext and a new option to control the
>          amount of padding added to headers.  The current default value is 0
>          and it is limited to between 0 and 1MiB (2^20).
> 
> Patch 2: Uses the new option in flacenc.  Previously the muxer wrote a fixed
>          8192 bytes of padding.
> 
> Patch 4: Uses the new option in id3v2 tags.  There is a comment there saying
>          that 10 bytes are written solve compatability problems with other
>          software.  Does this mean I should change the default padding value
>          from 0?  Should I clip the value in id3 tags to 10?
>          Comment quoted below.
>> adding an arbitrary amount of padding bytes at the end of the ID3 metadata
>> fixes cover art display for some software (iTunes, Traktor, Serato, Torq)
> 
> Changes to FATE results need doing.
> 
> James Darnley (4):
>   AVFormatContext: add metadata_header_padding field
>   lavf/flacenc: use metadata_header_padding
>   lavf/flacenc: fix comment after previous change
>   lavf/id3v2enc: use metadata_header_padding
> 
>  libavformat/aiffenc.c       |    2 +-
>  libavformat/avformat.h      |    7 +++++++
>  libavformat/flacenc.c       |   11 +++++++----
>  libavformat/id3v2.h         |    2 +-
>  libavformat/id3v2enc.c      |   11 +++++------
>  libavformat/mp3enc.c        |    4 ++--
>  libavformat/options_table.h |    1 +
>  7 files changed, 24 insertions(+), 14 deletions(-)

Bump lavf minor in the first patch.

> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index 8145325..d9d826a 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -77,6 +77,7 @@ static const AVOption avformat_options[] = {
>  {"skip_initial_bytes", "set number of bytes to skip before reading header and frames", OFFSET(skip_initial_bytes), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX-1, D},
>  {"correct_ts_overflow", "correct single timestamp overflows", OFFSET(correct_ts_overflow), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, D},
>  {"flush_packets", "enable flushing of the I/O context after each packet", OFFSET(flush_packets), AV_OPT_TYPE_INT, {.i64 = 1}, 0, 1, E},
> +{"metadata_header_padding", "set number of bytes to be written as padding in a metadata header", OFFSET(metadata_header_padding), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1048576 /* How large should this go?  Not all formats are equal. */, E},
>  {NULL},
>  };

-1, INT_MAX.
-1 for default value, 0 for no padding. Muxers can and should clip the value if needed.
Ideally look up the corresponding limit for both flac and id3v2.

> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index 87e9362..31b60d1 100644
> --- a/libavformat/flacenc.c
> +++ b/libavformat/flacenc.c
> @@ -64,7 +64,7 @@ static int flac_write_block_comment(AVIOContext *pb, AVDictionary **m,
>  
>  static int flac_write_header(struct AVFormatContext *s)
>  {
> -    int ret;
> +    int ret, padding;
>      AVCodecContext *codec = s->streams[0]->codec;
>  
>      if (s->nb_streams > 1) {
> @@ -76,11 +76,13 @@ static int flac_write_header(struct AVFormatContext *s)
>          return AVERROR(EINVAL);
>      }
>  
> +    padding = s->metadata_header_padding;
> +
>      ret = ff_flac_write_header(s->pb, codec, 0);
>      if (ret)
>          return ret;
>  
> -    ret = flac_write_block_comment(s->pb, &s->metadata, 0,
> +    ret = flac_write_block_comment(s->pb, &s->metadata, !padding,
>                                     codec->flags & CODEC_FLAG_BITEXACT);
>      if (ret)
>          return ret;
> @@ -89,7 +91,8 @@ static int flac_write_header(struct AVFormatContext *s)
>       * 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);
> +    if (padding)
> +        flac_write_block_padding(s->pb, padding, 1);
>  
>      return ret;
>  }

You need to keep the default of 8192 if there's no user provided value.
Same with the value in the id3v2 patch.

> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index 31b60d1..4eb581c 100644
> --- a/libavformat/flacenc.c
> +++ b/libavformat/flacenc.c
> @@ -90,7 +90,7 @@ static int flac_write_header(struct AVFormatContext *s)
>      /* 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 */
> +     * So just add the amount requested by the user. */

* So let's try the flac default of 8192 bytes, or the user 
  provided value */

>      if (padding)
>          flac_write_block_padding(s->pb, padding, 1);