[FFmpeg-devel] [PATCHv2 2/5] vorbis: handle special packets in the middle of a stream

[wm4]

On Fri, 25 Oct 2013 22:18:05 -0400
Ben Boeckel <mathstuf at gmail.com> wrote:

> This allows for updating metadata from new metadata packets in the
> middle of a stream (e.g., MPD streams). There still needs to be a signal
> that there *is* new metadata, but this is at least gets the data into a
> data structure.
> 
> Signed-off-by: Ben Boeckel <mathstuf at gmail.com>
> ---
>  libavcodec/vorbis_parser.c | 26 ++++++++++++++++++++++++--
>  libavcodec/vorbis_parser.h | 17 +++++++++++++++++
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vorbis_parser.c b/libavcodec/vorbis_parser.c
> index fcbecc8..1e2cab3 100644
> --- a/libavcodec/vorbis_parser.c
> +++ b/libavcodec/vorbis_parser.c
> @@ -201,8 +201,8 @@ int avpriv_vorbis_parse_extradata(AVCodecContext *avctx, VorbisParseContext *s)
>      return 0;
>  }
>  
> -int avpriv_vorbis_parse_frame(VorbisParseContext *s, const uint8_t *buf,
> -                              int buf_size)
> +int avpriv_vorbis_parse_frame_flags(VorbisParseContext *s, const uint8_t *buf,
> +                                    int buf_size, int *flags)
>  {
>      int duration = 0;
>  
> @@ -211,6 +211,22 @@ int avpriv_vorbis_parse_frame(VorbisParseContext *s, const uint8_t *buf,
>          int previous_blocksize = s->previous_blocksize;
>  
>          if (buf[0] & 1) {
> +            /* If the user doesn't care about special packets, it's a bad one. */
> +            if (!flags)
> +                goto bad_packet;
> +
> +            /* Set the flag for which kind of special packet it is. */
> +            if (buf[0] == 1)
> +                *flags |= VORBIS_FLAG_HEADER;
> +            else if (buf[0] == 3)
> +                *flags |= VORBIS_FLAG_COMMENT;
> +            else
> +                goto bad_packet;
> +
> +            /* Special packets have no duration. */
> +            return 0;
> +
> +bad_packet:
>              av_log(s->avctx, AV_LOG_ERROR, "Invalid packet\n");
>              return AVERROR_INVALIDDATA;
>          }
> @@ -234,6 +250,12 @@ int avpriv_vorbis_parse_frame(VorbisParseContext *s, const uint8_t *buf,
>      return duration;
>  }
>  
> +int avpriv_vorbis_parse_frame(VorbisParseContext *s, const uint8_t *buf,
> +                              int buf_size)
> +{
> +    return avpriv_vorbis_parse_frame_flags(s, buf, buf_size, NULL);
> +}
> +
>  void avpriv_vorbis_parse_reset(VorbisParseContext *s)
>  {
>      if (s->valid_extradata)
> diff --git a/libavcodec/vorbis_parser.h b/libavcodec/vorbis_parser.h
> index 101df5d..13a6cf4 100644
> --- a/libavcodec/vorbis_parser.h
> +++ b/libavcodec/vorbis_parser.h
> @@ -50,6 +50,23 @@ typedef struct VorbisParseContext {
>   */
>  int avpriv_vorbis_parse_extradata(AVCodecContext *avctx, VorbisParseContext *s);
>  
> +#define VORBIS_FLAG_HEADER  0x00000001
> +#define VORBIS_FLAG_COMMENT 0x00000002
> +
> +/**
> + * Get the duration for a Vorbis packet.
> + *
> + * avpriv_vorbis_parse_extradata() must have been successfully called prior to
> + * this in order for a correct duration to be returned.
> + *
> + * @param s        Vorbis parser context
> + * @param buf      buffer containing a Vorbis frame
> + * @param buf_size size of the buffer
> + * @param flags    flags for special frames (NULL for "don't care")
> + */
> +int avpriv_vorbis_parse_frame_flags(VorbisParseContext *s, const uint8_t *buf,
> +                                    int buf_size, int *flags);
> +
>  /**
>   * Get the duration for a Vorbis packet.
>   *

Looks ok, at least it doesn't change current behavior.

If you want to be pedantic, you could criticize that setting flags to
NULL instead of a dummy changes behavior, and
that avpriv_vorbis_parse_frame() relies on this.