diff mbox

[FFmpeg-devel,2/3] hevc: Support extradata changes

Message ID 20161108220328.7397-2-vittorio.giovara@gmail.com
State Accepted
Headers show

Commit Message

Vittorio Giovara Nov. 8, 2016, 10:03 p.m. UTC
Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
---
Applied review.
Please CC.
Vittorio

 libavcodec/hevc.c | 10 ++++++++++
 libavformat/mov.c |  4 ----
 2 files changed, 10 insertions(+), 4 deletions(-)

Comments

Vittorio Giovara Nov. 17, 2016, 3:38 p.m. UTC | #1
On Tue, Nov 8, 2016 at 5:03 PM, Vittorio Giovara
<vittorio.giovara@gmail.com> wrote:
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> ---
> Applied review.
> Please CC.
> Vittorio
>
>  libavcodec/hevc.c | 10 ++++++++++
>  libavformat/mov.c |  4 ----
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
> index 02fd606..4417f79 100644
> --- a/libavcodec/hevc.c
> +++ b/libavcodec/hevc.c
> @@ -3049,6 +3049,8 @@ static int hevc_decode_frame(AVCodecContext *avctx, void *data, int *got_output,
>                               AVPacket *avpkt)
>  {
>      int ret;
> +    int new_extradata_size;
> +    uint8_t *new_extradata;
>      HEVCContext *s = avctx->priv_data;
>
>      if (!avpkt->size) {
> @@ -3060,6 +3062,14 @@ static int hevc_decode_frame(AVCodecContext *avctx, void *data, int *got_output,
>          return 0;
>      }
>
> +    new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA,
> +                                            &new_extradata_size);
> +    if (new_extradata && new_extradata_size > 0) {
> +        ret = hevc_decode_extradata(s, new_extradata, new_extradata_size);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
>      s->ref = NULL;
>      ret    = decode_nal_units(s, avpkt->data, avpkt->size);
>      if (ret < 0)
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 2fc09b1..a2a688b 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2231,10 +2231,6 @@ static int mov_skip_multiple_stsd(MOVContext *c, AVIOContext *pb,
>          avio_skip(pb, size);
>          return 1;
>      }
> -    if ( codec_tag == AV_RL32("hvc1") ||
> -         codec_tag == AV_RL32("hev1")
> -    )
> -        av_log(c->fc, AV_LOG_WARNING, "Concatenated H.264 or H.265 might not play correctly.\n");
>
>      return 0;
>  }
> --
> 2.10.0
>

ping
Vittorio Giovara Nov. 28, 2016, 10:44 p.m. UTC | #2
On Thu, Nov 17, 2016 at 10:38 AM, Vittorio Giovara
<vittorio.giovara@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 5:03 PM, Vittorio Giovara
> <vittorio.giovara@gmail.com> wrote:
>> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
>> ---

Hi, if no further objections I'll push the set tomorrow.
           o hevc: Support extradata changes
           o hevc: Allow parsing external extradata buffers
Please CC.
Michael Niedermayer Nov. 29, 2016, 2:06 a.m. UTC | #3
On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> ---
> Applied review.
> Please CC.
> Vittorio
> 
>  libavcodec/hevc.c | 10 ++++++++++
>  libavformat/mov.c |  4 ----

please split this in 2 patches, the libavcodec one probably should
also have its version bumped as apps might want to depend on
a libavcodec with that feature

[...]
Vittorio Giovara Nov. 29, 2016, 3:03 a.m. UTC | #4
On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote:
>> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
>> ---
>> Applied review.
>> Please CC.
>> Vittorio
>>
>>  libavcodec/hevc.c | 10 ++++++++++
>>  libavformat/mov.c |  4 ----
>
> please split this in 2 patches, the libavcodec one probably should
> also have its version bumped as apps might want to depend on
> a libavcodec with that feature

ok for the version bumb, why splitting it 2 patches though?
Michael Niedermayer Nov. 29, 2016, 12:20 p.m. UTC | #5
On Mon, Nov 28, 2016 at 10:03:37PM -0500, Vittorio Giovara wrote:
> On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> > On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote:
> >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> >> ---
> >> Applied review.
> >> Please CC.
> >> Vittorio
> >>
> >>  libavcodec/hevc.c | 10 ++++++++++
> >>  libavformat/mov.c |  4 ----
> >
> > please split this in 2 patches, the libavcodec one probably should
> > also have its version bumped as apps might want to depend on
> > a libavcodec with that feature
> 
> ok for the version bumb, why splitting it 2 patches though?

making changes to 2 libs at the same time can mask bugs because
you cannot checkout and test the intermediate but in distributions
users can end up with one lib updated and the other not (within what
the dependancies and versions allow)
So i always suggest spliting non cosmetic changes into a patch per lib
unless i miss/forget

[...]
Vittorio Giovara Nov. 29, 2016, 3:53 p.m. UTC | #6
On Tue, Nov 29, 2016 at 7:20 AM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Mon, Nov 28, 2016 at 10:03:37PM -0500, Vittorio Giovara wrote:
>> On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer
>> <michael@niedermayer.cc> wrote:
>> > On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote:
>> >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
>> >> ---
>> >> Applied review.
>> >> Please CC.
>> >> Vittorio
>> >>
>> >>  libavcodec/hevc.c | 10 ++++++++++
>> >>  libavformat/mov.c |  4 ----
>> >
>> > please split this in 2 patches, the libavcodec one probably should
>> > also have its version bumped as apps might want to depend on
>> > a libavcodec with that feature
>>
>> ok for the version bumb, why splitting it 2 patches though?
>
> making changes to 2 libs at the same time can mask bugs because
> you cannot checkout and test the intermediate but in distributions
> users can end up with one lib updated and the other not (within what
> the dependancies and versions allow)
> So i always suggest spliting non cosmetic changes into a patch per lib
> unless i miss/forget

Well yes, but the changes in lavf only drop a log line, which is
useless because of this patch, so imo it makes sense to keep them in
the same diff. I'll split it if you insist though.
Thanks
Michael Niedermayer Nov. 29, 2016, 4:43 p.m. UTC | #7
On Tue, Nov 29, 2016 at 10:53:15AM -0500, Vittorio Giovara wrote:
> On Tue, Nov 29, 2016 at 7:20 AM, Michael Niedermayer
> <michael@niedermayer.cc> wrote:
> > On Mon, Nov 28, 2016 at 10:03:37PM -0500, Vittorio Giovara wrote:
> >> On Mon, Nov 28, 2016 at 9:06 PM, Michael Niedermayer
> >> <michael@niedermayer.cc> wrote:
> >> > On Tue, Nov 08, 2016 at 05:03:27PM -0500, Vittorio Giovara wrote:
> >> >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> >> >> ---
> >> >> Applied review.
> >> >> Please CC.
> >> >> Vittorio
> >> >>
> >> >>  libavcodec/hevc.c | 10 ++++++++++
> >> >>  libavformat/mov.c |  4 ----
> >> >
> >> > please split this in 2 patches, the libavcodec one probably should
> >> > also have its version bumped as apps might want to depend on
> >> > a libavcodec with that feature
> >>
> >> ok for the version bumb, why splitting it 2 patches though?
> >
> > making changes to 2 libs at the same time can mask bugs because
> > you cannot checkout and test the intermediate but in distributions
> > users can end up with one lib updated and the other not (within what
> > the dependancies and versions allow)
> > So i always suggest spliting non cosmetic changes into a patch per lib
> > unless i miss/forget
> 
> Well yes, but the changes in lavf only drop a log line, which is
> useless because of this patch, so imo it makes sense to keep them in
> the same diff. I'll split it if you insist though.

its ok to keep it on one patch if you prefer

[...]
diff mbox

Patch

diff --git a/libavcodec/hevc.c b/libavcodec/hevc.c
index 02fd606..4417f79 100644
--- a/libavcodec/hevc.c
+++ b/libavcodec/hevc.c
@@ -3049,6 +3049,8 @@  static int hevc_decode_frame(AVCodecContext *avctx, void *data, int *got_output,
                              AVPacket *avpkt)
 {
     int ret;
+    int new_extradata_size;
+    uint8_t *new_extradata;
     HEVCContext *s = avctx->priv_data;
 
     if (!avpkt->size) {
@@ -3060,6 +3062,14 @@  static int hevc_decode_frame(AVCodecContext *avctx, void *data, int *got_output,
         return 0;
     }
 
+    new_extradata = av_packet_get_side_data(avpkt, AV_PKT_DATA_NEW_EXTRADATA,
+                                            &new_extradata_size);
+    if (new_extradata && new_extradata_size > 0) {
+        ret = hevc_decode_extradata(s, new_extradata, new_extradata_size);
+        if (ret < 0)
+            return ret;
+    }
+
     s->ref = NULL;
     ret    = decode_nal_units(s, avpkt->data, avpkt->size);
     if (ret < 0)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 2fc09b1..a2a688b 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2231,10 +2231,6 @@  static int mov_skip_multiple_stsd(MOVContext *c, AVIOContext *pb,
         avio_skip(pb, size);
         return 1;
     }
-    if ( codec_tag == AV_RL32("hvc1") ||
-         codec_tag == AV_RL32("hev1")
-    )
-        av_log(c->fc, AV_LOG_WARNING, "Concatenated H.264 or H.265 might not play correctly.\n");
 
     return 0;
 }