[FFmpeg-devel] libavformat/matroskaenc.c: Write Tags element for WebM

Submitted by Ivan Janatra on Sept. 8, 2017, 3:59 p.m.

Details

Message ID 20170908155957.95147-1-janatra@google.com
State New
Headers show

Commit Message

Ivan Janatra Sept. 8, 2017, 3:59 p.m.
This is already supported per https://www.webmproject.org/docs/container/#Tags and https://github.com/nbirkbeck/matroska-specification/commit/28a54f991f118fff31fe6bfe256c2dfab46d00e5

Signed-off-by: Ivan Janatra <janatra@google.com>
---
 libavformat/matroskaenc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

James Almer Sept. 8, 2017, 10:33 p.m.
On 9/8/2017 12:59 PM, Ivan Janatra wrote:
> This is already supported per https://www.webmproject.org/docs/container/#Tags and https://github.com/nbirkbeck/matroska-specification/commit/28a54f991f118fff31fe6bfe256c2dfab46d00e5
> 
> Signed-off-by: Ivan Janatra <janatra@google.com>
> ---
>  libavformat/matroskaenc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 9cc7be352e..5b70fead87 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1988,12 +1988,12 @@ static int mkv_write_header(AVFormatContext *s)
>          ret = mkv_write_attachments(s);
>          if (ret < 0)
>              goto fail;
> -
> -        ret = mkv_write_tags(s);
> -        if (ret < 0)
> -            goto fail;
>      }
>  
> +    ret = mkv_write_tags(s);
> +    if (ret < 0)
> +        goto fail;
> +
>      if (!(s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
>          mkv_write_seekhead(pb, mkv);

This is incomplete, as it's allowing attachment and chapter tags to be
written to webm files, when TagChapterUID and TagAttachmentUID elements
are not allowed according to the spec you linked.

See https://ffmpeg.org/pipermail/ffmpeg-devel/2017-June/211863.html
I'll push that soon unless someone has comments.
Ivan Janatra Sept. 8, 2017, 10:51 p.m.
SGTM, I'll just wait for your patch then.

On Fri, Sep 8, 2017 at 3:33 PM, James Almer <jamrial@gmail.com> wrote:

> On 9/8/2017 12:59 PM, Ivan Janatra wrote:
> > This is already supported per https://www.webmproject.org/
> docs/container/#Tags and https://github.com/nbirkbeck/
> matroska-specification/commit/28a54f991f118fff31fe6bfe256c2dfab46d00e5
> >
> > Signed-off-by: Ivan Janatra <janatra@google.com>
> > ---
> >  libavformat/matroskaenc.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 9cc7be352e..5b70fead87 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -1988,12 +1988,12 @@ static int mkv_write_header(AVFormatContext *s)
> >          ret = mkv_write_attachments(s);
> >          if (ret < 0)
> >              goto fail;
> > -
> > -        ret = mkv_write_tags(s);
> > -        if (ret < 0)
> > -            goto fail;
> >      }
> >
> > +    ret = mkv_write_tags(s);
> > +    if (ret < 0)
> > +        goto fail;
> > +
> >      if (!(s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
> >          mkv_write_seekhead(pb, mkv);
>
> This is incomplete, as it's allowing attachment and chapter tags to be
> written to webm files, when TagChapterUID and TagAttachmentUID elements
> are not allowed according to the spec you linked.
>
> See https://ffmpeg.org/pipermail/ffmpeg-devel/2017-June/211863.html
> I'll push that soon unless someone has comments.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
James Almer Sept. 8, 2017, 11:05 p.m.
On 9/8/2017 7:51 PM, Ivan Janatra wrote:
> SGTM, I'll just wait for your patch then.
> 

Pushed, thanks.

Patch hide | download patch | download mbox

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 9cc7be352e..5b70fead87 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1988,12 +1988,12 @@  static int mkv_write_header(AVFormatContext *s)
         ret = mkv_write_attachments(s);
         if (ret < 0)
             goto fail;
-
-        ret = mkv_write_tags(s);
-        if (ret < 0)
-            goto fail;
     }
 
+    ret = mkv_write_tags(s);
+    if (ret < 0)
+        goto fail;
+
     if (!(s->pb->seekable & AVIO_SEEKABLE_NORMAL) && !mkv->is_live)
         mkv_write_seekhead(pb, mkv);