Add bwf muxer, wav file with bext chunk

Message ID 1300410015-9471-1-git-send-email-banan@ludd.ltu.se
State Superseded
Headers show

Commit Message

Benjamin Larsson March 18, 2011, 1 a.m.
From: Benjamin Larsson <benjamin@southpole.se>

---
 libavformat/Makefile     |    1 +
 libavformat/allformats.c |    1 +
 libavformat/wav.c        |   83 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 84 insertions(+), 1 deletions(-)

Comments

Kostya Shishkov March 18, 2011, 8:25 a.m. | #1
On Fri, Mar 18, 2011 at 02:00:15AM +0100, banan@ludd.ltu.se wrote:
> From: Benjamin Larsson <benjamin@southpole.se>
> ---
[...]
> diff --git a/libavformat/wav.c b/libavformat/wav.c
> index dea70fa..feacf61 100644
> --- a/libavformat/wav.c
> +++ b/libavformat/wav.c
[...]
> +
> +static void bwf_write_bext_chunk(AVFormatContext *s)
> +{
> +    int64_t bext;
> +    AVMetadataTag *tmp_tag;
> +    uint64_t time_reference = 0;
> +    bext = ff_start_tag(s->pb, "bext");

I'd put an empty line between declarations and assignment or merge them.

[...]
> +
> +    if (tmp_tag = av_metadata_get(s->metadata, "umid", NULL, 0)) {
> +        unsigned char umidpart_str[17] = {0};
> +        int i;
> +        uint64_t umidpart;
> +        int len = strlen(tmp_tag->value+2);
> +
> +        for (i=0 ; i<len/16 ; i++) {

weird formatting

[...]
> +
> +#if CONFIG_WAV_MUXER || CONFIG_BWF_MUXER

That's probably redundant - there are only two of these muxers, aren't there?


The rest if fine with me.
Anton Khirnov March 18, 2011, 8:38 a.m. | #2
On Fri, Mar 18, 2011 at 02:00:15AM +0100, banan@ludd.ltu.se wrote:
> From: Benjamin Larsson <benjamin@southpole.se>
> 
> ---
>  libavformat/Makefile     |    1 +
>  libavformat/allformats.c |    1 +
>  libavformat/wav.c        |   83 +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 84 insertions(+), 1 deletions(-)
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index f319031..4b95fc1 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -288,6 +288,7 @@ OBJS-$(CONFIG_VQF_DEMUXER)               += vqf.o
>  OBJS-$(CONFIG_W64_DEMUXER)               += wav.o riff.o pcm.o
>  OBJS-$(CONFIG_WAV_DEMUXER)               += wav.o riff.o pcm.o
>  OBJS-$(CONFIG_WAV_MUXER)                 += wav.o riff.o
> +OBJS-$(CONFIG_BWF_MUXER)                 += wav.o riff.o
>  OBJS-$(CONFIG_WC3_DEMUXER)               += wc3movie.o
>  OBJS-$(CONFIG_WEBM_MUXER)                += matroskaenc.o matroska.o \
>                                              riff.o isom.o avc.o \
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 0ff4b5a..375dfcf 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -215,6 +215,7 @@ void av_register_all(void)
>      REGISTER_DEMUXER  (VQF, vqf);
>      REGISTER_DEMUXER  (W64, w64);
>      REGISTER_MUXDEMUX (WAV, wav);
> +    REGISTER_MUXER    (BWF, bwf);
>      REGISTER_DEMUXER  (WC3, wc3);
>      REGISTER_MUXER    (WEBM, webm);
>      REGISTER_DEMUXER  (WSAUD, wsaud);
> diff --git a/libavformat/wav.c b/libavformat/wav.c
> index dea70fa..feacf61 100644
> --- a/libavformat/wav.c
> +++ b/libavformat/wav.c
> @@ -25,6 +25,8 @@
>  #include "avformat.h"
>  #include "pcm.h"
>  #include "riff.h"
> +#include "avio.h"
> +#include "avio_internal.h"
>  
>  typedef struct {
>      int64_t data;
> @@ -35,7 +37,66 @@ typedef struct {
>      int w64;
>  } WAVContext;
>  
> -#if CONFIG_WAV_MUXER
> +#if CONFIG_BWF_MUXER
> +extern AVOutputFormat ff_bwf_muxer;
> +
> +static inline void bwf_write_bext_string(AVFormatContext *s, const char *key, int maxlen)
> +{
> +    AVMetadataTag *tag;
> +    int len = 0;
> +
> +    if (tag = av_metadata_get(s->metadata, key, NULL, 0)) {
> +        len = strlen(tag->value);
> +        len = FFMIN(len, maxlen);
> +        avio_write(s->pb, tag->value, len);
> +    }
> +
> +    ffio_fill(s->pb, 0, maxlen - len);
> +}
> +
> +static void bwf_write_bext_chunk(AVFormatContext *s)
> +{
> +    int64_t bext;
> +    AVMetadataTag *tmp_tag;
> +    uint64_t time_reference = 0;
> +    bext = ff_start_tag(s->pb, "bext");
> +
> +    bwf_write_bext_string(s, "description", 256);
> +    bwf_write_bext_string(s, "originator", 32);
> +    bwf_write_bext_string(s, "originator_reference", 32);
> +    bwf_write_bext_string(s, "origination_date", 10);
> +    bwf_write_bext_string(s, "origination_time", 8);
> +
> +    if (tmp_tag = av_metadata_get(s->metadata, "time_reference", NULL, 0))
> +        time_reference = strtoll(tmp_tag->value, NULL, 10);
> +    avio_wl64(s->pb, time_reference);
> +    avio_wl16(s->pb, 1);  // set version to 1
> +
> +    if (tmp_tag = av_metadata_get(s->metadata, "umid", NULL, 0)) {
> +        unsigned char umidpart_str[17] = {0};
> +        int i;
> +        uint64_t umidpart;
> +        int len = strlen(tmp_tag->value+2);
> +
> +        for (i=0 ; i<len/16 ; i++) {
> +            memcpy(umidpart_str, tmp_tag->value+2+(i*16), 16);
> +            umidpart = strtoll(umidpart_str, NULL, 16);
> +            avio_wb64(s->pb, umidpart);
> +        }
> +        ffio_fill(s->pb, 0, 64-i*8);
> +    } else
> +        ffio_fill(s->pb, 0, 64); // zero UMID
> +
> +    ffio_fill(s->pb, 0, 190); // Reserved
> +
> +    if (tmp_tag = av_metadata_get(s->metadata, "coding_history", NULL, 0))
> +        avio_put_str(s->pb, tmp_tag->value);
> +
> +    ff_end_tag(s->pb, bext);
> +}
> +#endif/* CONFIG_BWF_MUXER */
> +
> +#if CONFIG_WAV_MUXER || CONFIG_BWF_MUXER
>  static int wav_write_header(AVFormatContext *s)
>  {
>      WAVContext *wav = s->priv_data;
> @@ -63,6 +124,11 @@ static int wav_write_header(AVFormatContext *s)
>          ff_end_tag(pb, fact);
>      }
>  
> +#if CONFIG_BWF_MUXER
> +    if (s->oformat == &ff_bwf_muxer)
> +        bwf_write_bext_chunk(s);
> +#endif
> +
>      av_set_pts_info(s->streams[0], 64, 1, s->streams[0]->codec->sample_rate);
>      wav->maxpts = wav->last_duration = 0;
>      wav->minpts = INT64_MAX;
> @@ -122,7 +188,9 @@ static int wav_write_trailer(AVFormatContext *s)
>      }
>      return 0;
>  }
> +#endif /* CONFIG_WAV_MUXER || CONFIG_BWF_MUXER */
>  
> +#if CONFIG_WAV_MUXER
>  AVOutputFormat ff_wav_muxer = {
>      "wav",
>      NULL_IF_CONFIG_SMALL("WAV format"),
> @@ -138,6 +206,19 @@ AVOutputFormat ff_wav_muxer = {
>  };
>  #endif /* CONFIG_WAV_MUXER */
>  
> +#if CONFIG_BWF_MUXER
> +AVOutputFormat ff_bwf_muxer = {
> +    .name           = "bwf",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Broadcast Wave format"),
> +    .extensions     = "wav,bwf",
> +    .priv_data_size = sizeof(WAVContext),
> +    .audio_codec    = CODEC_ID_PCM_S16LE,
> +    .write_header   = wav_write_header,
> +    .write_packet   = wav_write_packet,
> +    .write_trailer  = wav_write_trailer,
> +    .codec_tag      = (const AVCodecTag* const []){ff_codec_wav_tags, 0},
> +};
> +#endif /* CONFIG_BWF_MUXER */
>  
>  #if CONFIG_WAV_DEMUXER
>  
> -- 
> 1.7.1
> 

You're using only non-standard metadata tags here. Can't some of those
be mapped to standard tags (see the list in avformat.h). If not, would
it make sense for them to be added to the list?
Benjamin Larsson March 18, 2011, 10:10 a.m. | #3
> You're using only non-standard metadata tags here. Can't some of those
> be mapped to standard tags (see the list in avformat.h). If not, would
> it make sense for them to be added to the list?

Well the tag names are specified. I'm reluctant to change that as it
would not conform to the specification then.

MvH
Benjamin Larsson
Anton Khirnov March 18, 2011, 10:22 a.m. | #4
On Fri, Mar 18, 2011 at 11:10:14AM +0100, Benjamin Larsson wrote:
> 
> > You're using only non-standard metadata tags here. Can't some of those
> > be mapped to standard tags (see the list in avformat.h). If not, would
> > it make sense for them to be added to the list?
> 
> Well the tag names are specified. I'm reluctant to change that as it
> would not conform to the specification then.

We have a system for that in place. You make an AVMetadataConv table
(metadata.h) containing mappings between generic and format-specific tag
names and then you call ff_metadata_conv(AVMetadata, your_conv_table, NULL)
right before writing the tags and it'll convert the tags for you.

This way the users don't have to remember if the tag is named artist,
author, TPE1, LEAD_PERFORMER or whatever, they just use a generic tag
name and lavf does TheRightThing.
Benjamin Larsson March 18, 2011, 10:27 a.m. | #5
On 03/18/2011 11:22 AM, Anton Khirnov wrote:
> On Fri, Mar 18, 2011 at 11:10:14AM +0100, Benjamin Larsson wrote:
>>> You're using only non-standard metadata tags here. Can't some of those
>>> be mapped to standard tags (see the list in avformat.h). If not, would
>>> it make sense for them to be added to the list?
>> Well the tag names are specified. I'm reluctant to change that as it
>> would not conform to the specification then.
> We have a system for that in place. You make an AVMetadataConv table
> (metadata.h) containing mappings between generic and format-specific tag
> names and then you call ff_metadata_conv(AVMetadata, your_conv_table, NULL)
> right before writing the tags and it'll convert the tags for you.
>
> This way the users don't have to remember if the tag is named artist,
> author, TPE1, LEAD_PERFORMER or whatever, they just use a generic tag
> name and lavf does TheRightThing.

How neat. I'll look into it later.

MvH
Benjamin Larsson
Diego Biurrun March 23, 2011, 3:39 p.m. | #6
On Fri, Mar 18, 2011 at 02:00:15AM +0100, banan@ludd.ltu.se wrote:
> From: Benjamin Larsson <benjamin@southpole.se>
> 
> ---
>  libavformat/Makefile     |    1 +
>  libavformat/allformats.c |    1 +
>  libavformat/wav.c        |   83 +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 84 insertions(+), 1 deletions(-)

This is missing a doc and changelog update

> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -288,6 +288,7 @@ OBJS-$(CONFIG_VQF_DEMUXER)               += vqf.o
>  OBJS-$(CONFIG_WAV_DEMUXER)               += wav.o riff.o pcm.o
>  OBJS-$(CONFIG_WAV_MUXER)                 += wav.o riff.o
> +OBJS-$(CONFIG_BWF_MUXER)                 += wav.o riff.o
>  OBJS-$(CONFIG_WC3_DEMUXER)               += wc3movie.o

order

> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -215,6 +215,7 @@ void av_register_all(void)
>      REGISTER_DEMUXER  (W64, w64);
>      REGISTER_MUXDEMUX (WAV, wav);
> +    REGISTER_MUXER    (BWF, bwf);
>      REGISTER_DEMUXER  (WC3, wc3);

ditto

> --- a/libavformat/wav.c
> +++ b/libavformat/wav.c
> @@ -35,7 +37,66 @@ typedef struct {
> +
> +static inline void bwf_write_bext_string(AVFormatContext *s, const char *key, int maxlen)

extra good karma for breaking this long line

> +        for (i=0 ; i<len/16 ; i++) {
> +            memcpy(umidpart_str, tmp_tag->value+2+(i*16), 16);
> +    } else
> +        ffio_fill(s->pb, 0, 64); // zero UMID

Whitespace around operators would make the above lines more readable.

> @@ -138,6 +206,19 @@ AVOutputFormat ff_wav_muxer = {
>  
> +#if CONFIG_BWF_MUXER
> +AVOutputFormat ff_bwf_muxer = {
> +    .name           = "bwf",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Broadcast Wave format"),
> +    .extensions     = "wav,bwf",
> +    .priv_data_size = sizeof(WAVContext),
> +    .audio_codec    = CODEC_ID_PCM_S16LE,
> +    .write_header   = wav_write_header,
> +    .write_packet   = wav_write_packet,
> +    .write_trailer  = wav_write_trailer,
> +    .codec_tag      = (const AVCodecTag* const []){ff_codec_wav_tags, 0},
> +};
> +#endif /* CONFIG_BWF_MUXER */

Add this to the CONFIG_BWF_MUXER block above and save one #ifdef.

Diego

Patch

diff --git a/libavformat/Makefile b/libavformat/Makefile
index f319031..4b95fc1 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -288,6 +288,7 @@  OBJS-$(CONFIG_VQF_DEMUXER)               += vqf.o
 OBJS-$(CONFIG_W64_DEMUXER)               += wav.o riff.o pcm.o
 OBJS-$(CONFIG_WAV_DEMUXER)               += wav.o riff.o pcm.o
 OBJS-$(CONFIG_WAV_MUXER)                 += wav.o riff.o
+OBJS-$(CONFIG_BWF_MUXER)                 += wav.o riff.o
 OBJS-$(CONFIG_WC3_DEMUXER)               += wc3movie.o
 OBJS-$(CONFIG_WEBM_MUXER)                += matroskaenc.o matroska.o \
                                             riff.o isom.o avc.o \
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index 0ff4b5a..375dfcf 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -215,6 +215,7 @@  void av_register_all(void)
     REGISTER_DEMUXER  (VQF, vqf);
     REGISTER_DEMUXER  (W64, w64);
     REGISTER_MUXDEMUX (WAV, wav);
+    REGISTER_MUXER    (BWF, bwf);
     REGISTER_DEMUXER  (WC3, wc3);
     REGISTER_MUXER    (WEBM, webm);
     REGISTER_DEMUXER  (WSAUD, wsaud);
diff --git a/libavformat/wav.c b/libavformat/wav.c
index dea70fa..feacf61 100644
--- a/libavformat/wav.c
+++ b/libavformat/wav.c
@@ -25,6 +25,8 @@ 
 #include "avformat.h"
 #include "pcm.h"
 #include "riff.h"
+#include "avio.h"
+#include "avio_internal.h"
 
 typedef struct {
     int64_t data;
@@ -35,7 +37,66 @@  typedef struct {
     int w64;
 } WAVContext;
 
-#if CONFIG_WAV_MUXER
+#if CONFIG_BWF_MUXER
+extern AVOutputFormat ff_bwf_muxer;
+
+static inline void bwf_write_bext_string(AVFormatContext *s, const char *key, int maxlen)
+{
+    AVMetadataTag *tag;
+    int len = 0;
+
+    if (tag = av_metadata_get(s->metadata, key, NULL, 0)) {
+        len = strlen(tag->value);
+        len = FFMIN(len, maxlen);
+        avio_write(s->pb, tag->value, len);
+    }
+
+    ffio_fill(s->pb, 0, maxlen - len);
+}
+
+static void bwf_write_bext_chunk(AVFormatContext *s)
+{
+    int64_t bext;
+    AVMetadataTag *tmp_tag;
+    uint64_t time_reference = 0;
+    bext = ff_start_tag(s->pb, "bext");
+
+    bwf_write_bext_string(s, "description", 256);
+    bwf_write_bext_string(s, "originator", 32);
+    bwf_write_bext_string(s, "originator_reference", 32);
+    bwf_write_bext_string(s, "origination_date", 10);
+    bwf_write_bext_string(s, "origination_time", 8);
+
+    if (tmp_tag = av_metadata_get(s->metadata, "time_reference", NULL, 0))
+        time_reference = strtoll(tmp_tag->value, NULL, 10);
+    avio_wl64(s->pb, time_reference);
+    avio_wl16(s->pb, 1);  // set version to 1
+
+    if (tmp_tag = av_metadata_get(s->metadata, "umid", NULL, 0)) {
+        unsigned char umidpart_str[17] = {0};
+        int i;
+        uint64_t umidpart;
+        int len = strlen(tmp_tag->value+2);
+
+        for (i=0 ; i<len/16 ; i++) {
+            memcpy(umidpart_str, tmp_tag->value+2+(i*16), 16);
+            umidpart = strtoll(umidpart_str, NULL, 16);
+            avio_wb64(s->pb, umidpart);
+        }
+        ffio_fill(s->pb, 0, 64-i*8);
+    } else
+        ffio_fill(s->pb, 0, 64); // zero UMID
+
+    ffio_fill(s->pb, 0, 190); // Reserved
+
+    if (tmp_tag = av_metadata_get(s->metadata, "coding_history", NULL, 0))
+        avio_put_str(s->pb, tmp_tag->value);
+
+    ff_end_tag(s->pb, bext);
+}
+#endif/* CONFIG_BWF_MUXER */
+
+#if CONFIG_WAV_MUXER || CONFIG_BWF_MUXER
 static int wav_write_header(AVFormatContext *s)
 {
     WAVContext *wav = s->priv_data;
@@ -63,6 +124,11 @@  static int wav_write_header(AVFormatContext *s)
         ff_end_tag(pb, fact);
     }
 
+#if CONFIG_BWF_MUXER
+    if (s->oformat == &ff_bwf_muxer)
+        bwf_write_bext_chunk(s);
+#endif
+
     av_set_pts_info(s->streams[0], 64, 1, s->streams[0]->codec->sample_rate);
     wav->maxpts = wav->last_duration = 0;
     wav->minpts = INT64_MAX;
@@ -122,7 +188,9 @@  static int wav_write_trailer(AVFormatContext *s)
     }
     return 0;
 }
+#endif /* CONFIG_WAV_MUXER || CONFIG_BWF_MUXER */
 
+#if CONFIG_WAV_MUXER
 AVOutputFormat ff_wav_muxer = {
     "wav",
     NULL_IF_CONFIG_SMALL("WAV format"),
@@ -138,6 +206,19 @@  AVOutputFormat ff_wav_muxer = {
 };
 #endif /* CONFIG_WAV_MUXER */
 
+#if CONFIG_BWF_MUXER
+AVOutputFormat ff_bwf_muxer = {
+    .name           = "bwf",
+    .long_name      = NULL_IF_CONFIG_SMALL("Broadcast Wave format"),
+    .extensions     = "wav,bwf",
+    .priv_data_size = sizeof(WAVContext),
+    .audio_codec    = CODEC_ID_PCM_S16LE,
+    .write_header   = wav_write_header,
+    .write_packet   = wav_write_packet,
+    .write_trailer  = wav_write_trailer,
+    .codec_tag      = (const AVCodecTag* const []){ff_codec_wav_tags, 0},
+};
+#endif /* CONFIG_BWF_MUXER */
 
 #if CONFIG_WAV_DEMUXER