[FFmpeg-devel] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

Submitted by Michael Niedermayer on March 8, 2017, 1:57 a.m.

Details

Message ID 20170308015750.28592-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer March 8, 2017, 1:57 a.m.
This allows to test or use  the code without av_packet_split_side_data() or
allows applications to separate the side data handling out not
running it automatically.

It also makes it easier to change the default behavior

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/avcodec.h |  5 +++++
 libavcodec/utils.c   | 12 ++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

Comments

wm4 March 8, 2017, 6:57 a.m.
On Wed,  8 Mar 2017 02:57:50 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> This allows to test or use  the code without av_packet_split_side_data() or
> allows applications to separate the side data handling out not
> running it automatically.
> 
> It also makes it easier to change the default behavior
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/avcodec.h |  5 +++++
>  libavcodec/utils.c   | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index af054f3194..92e930249e 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -910,6 +910,11 @@ typedef struct RcOverride{
>   */
>  #define AV_CODEC_FLAG2_FAST           (1 <<  0)
>  /**
> + * Do not split side data.
> + * @see AVFMT_FLAG_KEEP_SIDE_DATA
> + */
> +#define AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA (1 << 1)
> +/**
>   * Skip bitstream encoding.
>   */
>  #define AV_CODEC_FLAG2_NO_OUTPUT      (1 <<  2)
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index db3adb18d4..94278c6950 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2250,7 +2250,8 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>  
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
>          (avctx->active_thread_type & FF_THREAD_FRAME)) {
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2356,7 +2357,8 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
>          uint8_t discard_reason = 0;
>          // copy to ensure we do not change avpkt
>          AVPacket tmp = *avpkt;
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2669,7 +2671,8 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
>          AVPacket pkt_recoded;
>          AVPacket tmp = *avpkt;
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          //apply_param_change(avctx, &tmp);
>  
>          if (did_split) {
> @@ -2860,7 +2863,8 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>      if (avctx->codec->send_packet) {
>          if (avpkt) {
>              AVPacket tmp = *avpkt;
> -            int did_split = av_packet_split_side_data(&tmp);
> +            int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                            0 : av_packet_split_side_data(&tmp);
>              ret = apply_param_change(avctx, &tmp);
>              if (ret >= 0)
>                  ret = avctx->codec->send_packet(avctx, &tmp);

I don't understand, what's the purpose?

IMO this merge/split side data is a pointless waste of resources and
should be removed.
wm4 March 8, 2017, 8:32 a.m.
On Wed,  8 Mar 2017 02:57:50 +0100
Michael Niedermayer <michael@niedermayer.cc> wrote:

> This allows to test or use  the code without av_packet_split_side_data() or
> allows applications to separate the side data handling out not
> running it automatically.
> 
> It also makes it easier to change the default behavior
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/avcodec.h |  5 +++++
>  libavcodec/utils.c   | 12 ++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index af054f3194..92e930249e 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -910,6 +910,11 @@ typedef struct RcOverride{
>   */
>  #define AV_CODEC_FLAG2_FAST           (1 <<  0)
>  /**
> + * Do not split side data.
> + * @see AVFMT_FLAG_KEEP_SIDE_DATA
> + */
> +#define AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA (1 << 1)
> +/**
>   * Skip bitstream encoding.
>   */
>  #define AV_CODEC_FLAG2_NO_OUTPUT      (1 <<  2)
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index db3adb18d4..94278c6950 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2250,7 +2250,8 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>  
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
>          (avctx->active_thread_type & FF_THREAD_FRAME)) {
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2356,7 +2357,8 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
>          uint8_t discard_reason = 0;
>          // copy to ensure we do not change avpkt
>          AVPacket tmp = *avpkt;
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2669,7 +2671,8 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
>          AVPacket pkt_recoded;
>          AVPacket tmp = *avpkt;
> -        int did_split = av_packet_split_side_data(&tmp);
> +        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                        0 : av_packet_split_side_data(&tmp);
>          //apply_param_change(avctx, &tmp);
>  
>          if (did_split) {
> @@ -2860,7 +2863,8 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>      if (avctx->codec->send_packet) {
>          if (avpkt) {
>              AVPacket tmp = *avpkt;
> -            int did_split = av_packet_split_side_data(&tmp);
> +            int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +                            0 : av_packet_split_side_data(&tmp);
>              ret = apply_param_change(avctx, &tmp);
>              if (ret >= 0)
>                  ret = avctx->codec->send_packet(avctx, &tmp);

PS: while I don't see any immediate use, it could be good for
compatibility. We could set this flag by default for a while until side
data splitting has been eliminated from libavformat. This could be
useful to identify broken files where merged side data was muxed into
files (e.g. if someone demuxed with libavformat and muxed with his own
code, without setting the keep side data flag).

So LGTM.

Is there even any subtitle side data? Probably not, so that code could
probably be removed immediately.
Clément Bœsch March 8, 2017, 8:50 a.m.
On Wed, Mar 08, 2017 at 09:32:54AM +0100, wm4 wrote:
[...]
> Is there even any subtitle side data?

Yes, stuff like subrip coordinates or misc vtt stuff.

Patch hide | download patch | download mbox

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index af054f3194..92e930249e 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -910,6 +910,11 @@  typedef struct RcOverride{
  */
 #define AV_CODEC_FLAG2_FAST           (1 <<  0)
 /**
+ * Do not split side data.
+ * @see AVFMT_FLAG_KEEP_SIDE_DATA
+ */
+#define AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA (1 << 1)
+/**
  * Skip bitstream encoding.
  */
 #define AV_CODEC_FLAG2_NO_OUTPUT      (1 <<  2)
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index db3adb18d4..94278c6950 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2250,7 +2250,8 @@  int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
 
     if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
         (avctx->active_thread_type & FF_THREAD_FRAME)) {
-        int did_split = av_packet_split_side_data(&tmp);
+        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+                        0 : av_packet_split_side_data(&tmp);
         ret = apply_param_change(avctx, &tmp);
         if (ret < 0)
             goto fail;
@@ -2356,7 +2357,8 @@  int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
         uint8_t discard_reason = 0;
         // copy to ensure we do not change avpkt
         AVPacket tmp = *avpkt;
-        int did_split = av_packet_split_side_data(&tmp);
+        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+                        0 : av_packet_split_side_data(&tmp);
         ret = apply_param_change(avctx, &tmp);
         if (ret < 0)
             goto fail;
@@ -2669,7 +2671,8 @@  int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
     if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
         AVPacket pkt_recoded;
         AVPacket tmp = *avpkt;
-        int did_split = av_packet_split_side_data(&tmp);
+        int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+                        0 : av_packet_split_side_data(&tmp);
         //apply_param_change(avctx, &tmp);
 
         if (did_split) {
@@ -2860,7 +2863,8 @@  int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     if (avctx->codec->send_packet) {
         if (avpkt) {
             AVPacket tmp = *avpkt;
-            int did_split = av_packet_split_side_data(&tmp);
+            int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+                            0 : av_packet_split_side_data(&tmp);
             ret = apply_param_change(avctx, &tmp);
             if (ret >= 0)
                 ret = avctx->codec->send_packet(avctx, &tmp);