diff mbox series

[FFmpeg-devel,05/20] avformat/matroskaenc: Check return value of ff_isom_write_hvcc()

Message ID 20200101005837.11356-6-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series Matroska muxer patches | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 1, 2020, 12:58 a.m. UTC
The Matroska muxer currently does not check the return value of
ff_isom_write_hvcc(), the function used to write mp4-style
HEVC-extradata as Matroska also uses it. This was intentionally done in
7a5356c72 to allow remuxing from mpeg-ts.

But if ff_isom_write_hvcc() fails, it has not output anything and the
file ends up without CodecPrivate and, if the input was Annex B, with
Annex B data, which is against the spec. So check the return value
again.

The underlying issue of not having extradata seems to have been fixed by
the introduction of the extract_extradata bitstream filter.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/matroskaenc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

mypopy@gmail.com Jan. 1, 2020, 4:42 a.m. UTC | #1
On Wed, Jan 1, 2020 at 8:59 AM Andreas Rheinhardt
<andreas.rheinhardt@gmail.com> wrote:
>
> The Matroska muxer currently does not check the return value of
> ff_isom_write_hvcc(), the function used to write mp4-style
> HEVC-extradata as Matroska also uses it. This was intentionally done in
> 7a5356c72 to allow remuxing from mpeg-ts.
>
> But if ff_isom_write_hvcc() fails, it has not output anything and the
> file ends up without CodecPrivate and, if the input was Annex B, with
> Annex B data, which is against the spec. So check the return value
> again.
>
> The underlying issue of not having extradata seems to have been fixed by
> the introduction of the extract_extradata bitstream filter.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/matroskaenc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 76c38b8d09..8cea829b31 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -748,9 +748,8 @@ static int mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
>          return ff_isom_write_avcc(dyn_cp, par->extradata,
>                                    par->extradata_size);
>      case AV_CODEC_ID_HEVC:
> -        ff_isom_write_hvcc(dyn_cp, par->extradata,
> -                           par->extradata_size, 0);
> -        return 0;
> +        return ff_isom_write_hvcc(dyn_cp, par->extradata,
> +                                  par->extradata_size, 0);
>      case AV_CODEC_ID_AV1:
>          if (par->extradata_size)
>              return ff_isom_write_av1c(dyn_cp, par->extradata,
> --
> 2.20.1
LGTM
Michael Niedermayer Jan. 1, 2020, 3:31 p.m. UTC | #2
On Wed, Jan 01, 2020 at 12:42:51PM +0800, mypopy@gmail.com wrote:
> On Wed, Jan 1, 2020 at 8:59 AM Andreas Rheinhardt
> <andreas.rheinhardt@gmail.com> wrote:
> >
> > The Matroska muxer currently does not check the return value of
> > ff_isom_write_hvcc(), the function used to write mp4-style
> > HEVC-extradata as Matroska also uses it. This was intentionally done in
> > 7a5356c72 to allow remuxing from mpeg-ts.
> >
> > But if ff_isom_write_hvcc() fails, it has not output anything and the
> > file ends up without CodecPrivate and, if the input was Annex B, with
> > Annex B data, which is against the spec. So check the return value
> > again.
> >
> > The underlying issue of not having extradata seems to have been fixed by
> > the introduction of the extract_extradata bitstream filter.
> >
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> > ---
> >  libavformat/matroskaenc.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> > index 76c38b8d09..8cea829b31 100644
> > --- a/libavformat/matroskaenc.c
> > +++ b/libavformat/matroskaenc.c
> > @@ -748,9 +748,8 @@ static int mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
> >          return ff_isom_write_avcc(dyn_cp, par->extradata,
> >                                    par->extradata_size);
> >      case AV_CODEC_ID_HEVC:
> > -        ff_isom_write_hvcc(dyn_cp, par->extradata,
> > -                           par->extradata_size, 0);
> > -        return 0;
> > +        return ff_isom_write_hvcc(dyn_cp, par->extradata,
> > +                                  par->extradata_size, 0);
> >      case AV_CODEC_ID_AV1:
> >          if (par->extradata_size)
> >              return ff_isom_write_av1c(dyn_cp, par->extradata,
> > --
> > 2.20.1
> LGTM

will apply

thx

[...]
diff mbox series

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 76c38b8d09..8cea829b31 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -748,9 +748,8 @@  static int mkv_write_native_codecprivate(AVFormatContext *s, AVIOContext *pb,
         return ff_isom_write_avcc(dyn_cp, par->extradata,
                                   par->extradata_size);
     case AV_CODEC_ID_HEVC:
-        ff_isom_write_hvcc(dyn_cp, par->extradata,
-                           par->extradata_size, 0);
-        return 0;
+        return ff_isom_write_hvcc(dyn_cp, par->extradata,
+                                  par->extradata_size, 0);
     case AV_CODEC_ID_AV1:
         if (par->extradata_size)
             return ff_isom_write_av1c(dyn_cp, par->extradata,