Re: [FFmpeg-devel] [PATCH] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

[Clément Bœsch]

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.

-- 
Clément B.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

[wm4]

On Wed,  8 Mar 2017 02:57:50 +0100
Michael Niedermayer  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 
> ---
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  //apply_param_change(avctx, );
>  
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  if (ret >= 0)
>  ret = avctx->codec->send_packet(avctx, );

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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

[wm4]

On Wed,  8 Mar 2017 02:57:50 +0100
Michael Niedermayer  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 
> ---
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  //apply_param_change(avctx, );
>  
>  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();
> +int did_split = (avctx->flags2 & 
> AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
> +0 : av_packet_split_side_data();
>  ret = apply_param_change(avctx, );
>  if (ret >= 0)
>  ret = avctx->codec->send_packet(avctx, );

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

IMO this merge/split side data is a pointless waste of resources and
should be removed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] avcodec: Add AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA

[Michael Niedermayer]

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 
---
 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();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 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();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 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();
+int did_split = (avctx->flags2 & AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 //apply_param_change(avctx, );
 
 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();
+int did_split = (avctx->flags2 & 
AV_CODEC_FLAG2_DONT_SPLIT_SIDE_DATA) ?
+0 : av_packet_split_side_data();
 ret = apply_param_change(avctx, );
 if (ret >= 0)
 ret = avctx->codec->send_packet(avctx, );
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel