date:20200427

Re: [FFmpeg-devel] [PATCH v4 1/9] lavc/libopenh264enc: Add default qmin/qmax support

2020-04-27 Thread Fu, Linjie

> From: Martin Storsjö 
> Sent: Tuesday, April 28, 2020 03:27
> 
> If qmax/qmin < 0, i.e. wasn't specified by the user, wouldn't it be better
> to not touch param.iMax/MinQp at all (and use the default value of the
> library, which may change between versions), instead of overriding it with
> a value hardcoded here?
> 
Okay, this seems more natural if the recommended QP range varies between
versions, though one of my original purposes is to avoid the warning in default
situation for changing the QP inside libopenh264 library.

Updated locally as suggested, and thanks for the review for the whole set.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/2] avformat/vividas: simplify, use av_rescale_q() instead

2020-04-27 Thread lance . lmwang

From: Limin Wang 

note it'll cause a small difference in accuracy for the pts, please see the 
testing result below:
 $ wget 
http://samples.ffmpeg.org/archive/all/unknown+unknown+unknown+unknown+5029_paramount_en_1250.viv
 $ ./ffmpeg -t 0.04 -i 
./unknown+unknown+unknown+unknown+5029_paramount_en_1250.viv -f null -
 old:
 pts: 522
 pts: 1044
 pts: 1567
 pts: 3918
 pts: 8097
 pts: 12277
 pts: 16457
 ...

 new:
 pts: 522
 pts: 1045
 pts: 1567
 pts: 3918
 pts: 8098
 pts: 12278
 pts: 16457
 ...

Signed-off-by: Limin Wang 
---
 libavformat/vividas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/vividas.c b/libavformat/vividas.c
index 4f54a4302e..b0f9f35ac2 100644
--- a/libavformat/vividas.c
+++ b/libavformat/vividas.c
@@ -646,7 +646,7 @@ static int viv_read_packet(AVFormatContext *s,
 pkt->stream_index = 1;
 astream = s->streams[pkt->stream_index];
 
-pkt->pts = av_rescale(viv->audio_sample, astream->time_base.den, 
astream->time_base.num) / astream->codecpar->sample_rate;
+pkt->pts = av_rescale_q(viv->audio_sample, av_make_q(1, 
astream->codecpar->sample_rate), astream->time_base);
 viv->audio_sample += 
viv->audio_subpackets[viv->current_audio_subpacket].pcm_bytes / 2 / 
astream->codecpar->channels;
 pkt->flags |= AV_PKT_FLAG_KEY;
 viv->current_audio_subpacket++;
-- 
2.21.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avformat/wavenc: simplify, use av_rescale_q() instead

2020-04-27 Thread lance . lmwang

From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavformat/wavenc.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavformat/wavenc.c b/libavformat/wavenc.c
index f6f5710802..1027f107ee 100644
--- a/libavformat/wavenc.c
+++ b/libavformat/wavenc.c
@@ -434,10 +434,9 @@ static int wav_write_trailer(AVFormatContext *s)
"Filesize %"PRId64" invalid for wav, output file will be 
broken\n",
file_size);
 }
-
-number_of_samples = av_rescale(wav->maxpts - wav->minpts + 
wav->last_duration,
-   s->streams[0]->codecpar->sample_rate * 
(int64_t)s->streams[0]->time_base.num,
-   s->streams[0]->time_base.den);
+number_of_samples = av_rescale_q(wav->maxpts - wav->minpts + 
wav->last_duration,
+   s->streams[0]->time_base,
+   av_make_q(1, 
s->streams[0]->codecpar->sample_rate));
 
 if(s->streams[0]->codecpar->codec_tag != 0x01) {
 /* Update num_samps in fact chunk */
-- 
2.21.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 5/6] avcodec/pcm_rechunk_bsf: add bitstream filter to rechunk pcm audio

2020-04-27 Thread Andreas Rheinhardt

Marton Balint:
> 
> 
> On Mon, 27 Apr 2020, Marton Balint wrote:
> 
>>
>> On Sun, 26 Apr 2020, Andreas Rheinhardt wrote:
>>
> 
> [...]
> 
>>>
>>> And this filter does more than just repacketizing the samples: It also
>>> discards the timing of its input and makes up completely new timestamps
>>> and durations. This needs to be documented.
>>
>> Ok, will do.
> 
> I have given this some additional thought. Maybe it is better to keep
> the original timestamps instead of creating new. Keeping the start
> timestamp should definitely be supported by default and to make this
> filter analogous to af_asetnsamples, I am inclined to keep other
> timestamps as well. The code does not even look ugly, and
> retimeinterleave does not care about the timestamps anyway, just the
> durations. So I think I will resubmit a new version which will make an
> effort to keep the original timestamps. We can make it switchable if
> later it turns out that regenerating would be better.
> 
Sounds very good. In fact, the only reason I was ok with that is that I
thought this bsf to be a helper for retimeinterleave and I had the
impression that retimeinterleave depends on the timestamps being set in
the way the earlier version set them.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 5/6] avcodec/pcm_rechunk_bsf: add bitstream filter to rechunk pcm audio

2020-04-27 Thread Marton Balint




On Mon, 27 Apr 2020, Marton Balint wrote:



On Sun, 26 Apr 2020, Andreas Rheinhardt wrote:



[...]



And this filter does more than just repacketizing the samples: It also
discards the timing of its input and makes up completely new timestamps
and durations. This needs to be documented.


Ok, will do.


I have given this some additional thought. Maybe it is better to keep the 
original timestamps instead of creating new. Keeping the start timestamp 
should definitely be supported by default and to make this filter 
analogous to af_asetnsamples, I am inclined to keep other timestamps as 
well. The code does not even look ugly, and retimeinterleave does not care 
about the timestamps anyway, just the durations. So I think I will 
resubmit a new version which will make an effort to keep the original 
timestamps. We can make it switchable if later it turns out that 
regenerating would be better.


Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 2/3] avcodec/v4l2_m2m_enc: Enable frame level rate control by default

2020-04-27 Thread Andriy Gelman

On Sun, 23. Feb 13:47, Andriy Gelman wrote:
> From: Andriy Gelman 
> 
> Without this setting, bitrate and qmin/qmax options have no
> affect on the s5p-mfc hardware encoder.
> 
> Signed-off-by: Andriy Gelman 
> ---
>  libavcodec/v4l2_m2m_enc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> index 859feb7bde7..0098d5314bb 100644
> --- a/libavcodec/v4l2_m2m_enc.c
> +++ b/libavcodec/v4l2_m2m_enc.c
> @@ -179,6 +179,7 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
>  /* set ext ctrls */
>  v4l2_set_ext_ctrl(s, MPEG_CID(HEADER_MODE), 
> MPEG_VIDEO(HEADER_MODE_SEPARATE), "header mode", 0);
>  v4l2_set_ext_ctrl(s, MPEG_CID(BITRATE) , avctx->bit_rate, "bit rate", 1);
> +v4l2_set_ext_ctrl(s, MPEG_CID(FRAME_RC_ENABLE), 1, "frame level rate 
> control", 0);
>  v4l2_set_ext_ctrl(s, MPEG_CID(GOP_SIZE), avctx->gop_size,"gop size", 1);
>  
>  av_log(avctx, AV_LOG_DEBUG,
> -- 
> 2.25.0
> 

This patch was reviewed by Ming Qian :
> Lgtm,
> Would you please set bitrate mode to cbr by default too, for some driver, if 
> the default mode is vbr, the rc may be not token effect

I'll apply this evening if no one objects.  

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] Re: [PATCH v4 1/3] avcodec/v4l2_m2m_enc: Reduce log verbosity for some params

2020-04-27 Thread Andriy Gelman

On Thu, 09. Apr 07:17, Ming Qian wrote:
> lgtm

Thanks, will apply this later this evening if no one objects.

--
Andriy

> 
> 
> 
> Best regards,
> Ming Qian
> Tel#:86-512-6805-6630
> 
> 
> -Original Message-
> From: ffmpeg-devel  On Behalf Of Andriy 
> Gelman
> Sent: Saturday, March 14, 2020 10:32 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [EXT] Re: [FFmpeg-devel] [PATCH v4 1/3] avcodec/v4l2_m2m_enc: Reduce 
> log verbosity for some params
> 
> Caution: EXT Email
> 
> On Sun, 23. Feb 13:47, Andriy Gelman wrote:
> > From: Andriy Gelman 
> >
> > Currently the user gets unhelpful warnings when some default 
> > parameters are not supported by the device. The verbosity of these log 
> > messages has been changed to AV_LOG_DEBUG.
> >
> > Signed-off-by: Andriy Gelman 
> > ---
> >  libavcodec/v4l2_m2m_enc.c | 36 +---
> >  1 file changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c 
> > index c9f1741bfd0..859feb7bde7 100644
> > --- a/libavcodec/v4l2_m2m_enc.c
> > +++ b/libavcodec/v4l2_m2m_enc.c
> > @@ -47,7 +47,7 @@ static inline void v4l2_set_timeperframe(V4L2m2mContext 
> > *s, unsigned int num, un
> >  av_log(s->avctx, AV_LOG_WARNING, "Failed to set 
> > timeperframe");  }
> >
> > -static inline void v4l2_set_ext_ctrl(V4L2m2mContext *s, unsigned int 
> > id, signed int value, const char *name)
> > +static inline void v4l2_set_ext_ctrl(V4L2m2mContext *s, unsigned int 
> > +id, signed int value, const char *name, int log_warning)
> >  {
> >  struct v4l2_ext_controls ctrls = { { 0 } };
> >  struct v4l2_ext_control ctrl = { 0 }; @@ -62,12 +62,13 @@ static 
> > inline void v4l2_set_ext_ctrl(V4L2m2mContext *s, unsigned int id, signed
> >  ctrl.id = id;
> >
> >  if (ioctl(s->fd, VIDIOC_S_EXT_CTRLS, ) < 0)
> > -av_log(s->avctx, AV_LOG_WARNING, "Failed to set %s: %s\n", name, 
> > strerror(errno));
> > +av_log(s->avctx, log_warning || errno != EINVAL ? AV_LOG_WARNING : 
> > AV_LOG_DEBUG,
> > +   "Failed to set %s: %s\n", name, strerror(errno));
> >  else
> >  av_log(s->avctx, AV_LOG_DEBUG, "Encoder: %s = %d\n", name, 
> > value);  }
> >
> > -static inline int v4l2_get_ext_ctrl(V4L2m2mContext *s, unsigned int 
> > id, signed int *value, const char *name)
> > +static inline int v4l2_get_ext_ctrl(V4L2m2mContext *s, unsigned int 
> > +id, signed int *value, const char *name, int log_warning)
> >  {
> >  struct v4l2_ext_controls ctrls = { { 0 } };
> >  struct v4l2_ext_control ctrl = { 0 }; @@ -83,7 +84,8 @@ static 
> > inline int v4l2_get_ext_ctrl(V4L2m2mContext *s, unsigned int id, 
> > signed i
> >
> >  ret = ioctl(s->fd, VIDIOC_G_EXT_CTRLS, );
> >  if (ret < 0) {
> > -av_log(s->avctx, AV_LOG_WARNING, "Failed to get %s\n", name);
> > +av_log(s->avctx, log_warning || errno != EINVAL ? AV_LOG_WARNING : 
> > AV_LOG_DEBUG,
> > +   "Failed to get %s\n", name);
> >  return ret;
> >  }
> >
> > @@ -145,8 +147,8 @@ static int v4l2_check_b_frame_support(V4L2m2mContext *s)
> >  if (s->avctx->max_b_frames)
> >  av_log(s->avctx, AV_LOG_WARNING, "Encoder does not support 
> > b-frames yet\n");
> >
> > -v4l2_set_ext_ctrl(s, MPEG_CID(B_FRAMES), 0, "number of B-frames");
> > -v4l2_get_ext_ctrl(s, MPEG_CID(B_FRAMES), >avctx->max_b_frames, 
> > "number of B-frames");
> > +v4l2_set_ext_ctrl(s, MPEG_CID(B_FRAMES), 0, "number of B-frames", 0);
> > +v4l2_get_ext_ctrl(s, MPEG_CID(B_FRAMES), >avctx->max_b_frames, 
> > + "number of B-frames", 0);
> >  if (s->avctx->max_b_frames == 0)
> >  return 0;
> >
> > @@ -175,9 +177,9 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >  v4l2_set_timeperframe(s, avctx->framerate.num, 
> > avctx->framerate.den);
> >
> >  /* set ext ctrls */
> > -v4l2_set_ext_ctrl(s, MPEG_CID(HEADER_MODE), 
> > MPEG_VIDEO(HEADER_MODE_SEPARATE), "header mode");
> > -v4l2_set_ext_ctrl(s, MPEG_CID(BITRATE) , avctx->bit_rate, "bit rate");
> > -v4l2_set_ext_ctrl(s, MPEG_CID(GOP_SIZE), avctx->gop_size,"gop size");
> > +v4l2_set_ext_ctrl(s, MPEG_CID(HEADER_MODE), 
> > MPEG_VIDEO(HEADER_MODE_SEPARATE), "header mode", 0);
> > +v4l2_set_ext_ctrl(s, MPEG_CID(BITRATE) , avctx->bit_rate, "bit rate", 
> > 1);
> > +v4l2_set_ext_ctrl(s, MPEG_CID(GOP_SIZE), avctx->gop_size,"gop 
> > + size", 1);
> >
> >  av_log(avctx, AV_LOG_DEBUG,
> >  "Encoder Context: id (%d), profile (%d), frame rate(%d/%d), number 
> > b-frames (%d), "
> > @@ -187,26 +189,30 @@ static int v4l2_prepare_encoder(V4L2m2mContext 
> > *s)
> >
> >  switch (avctx->codec_id) {
> >  case AV_CODEC_ID_H264:
> > +if (avctx->profile != FF_PROFILE_UNKNOWN) {
> >  val = v4l2_h264_profile_from_ff(avctx->profile);
> >  if (val < 0)
> >  av_log(avctx, AV_LOG_WARNING, "h264 profile not found\n");
> >  else
> > -

Re: [FFmpeg-devel] [PATCH] swscale/vscale: Increase type strictness

2020-04-27 Thread Andreas Rheinhardt

Michael Niedermayer:
> On Sun, Apr 26, 2020 at 09:49:57AM +0200, Andreas Rheinhardt wrote:
>> libswscale/vscale.c makes extensive use of function pointers and in
>> doing so it converts these function pointers to and from a pointer to
>> void. Yet this is actually against the C standard:
>> C90 only guarantees that one can convert a pointer to any incomplete
>> type or object type to void* and back with the result comparing equal
>> to the original which makes pointers to void generic pointers to
>> incomplete or object type. Yet C90 lacks a generic function pointer
>> type.
>> C99 additionally guarantees that a pointer to a function of one type may
>> be converted to a pointer to a function of another type and when
>> converting back with the original and the result comparing equal.
>> This makes any function pointer type a generic function pointer type.
>> Yet even this does not make pointers to void generic function pointers.
>>
>> Both GCC and Clang emit warnings for this when in pedantic mode.
>>
>> This commit fixes this by using a union that can hold one member of any
>> of the required function pointer types to store the function pointer.
>> This works even for C90.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libswscale/vscale.c | 51 ++---
>>  1 file changed, 29 insertions(+), 22 deletions(-)
> 
> patch ok
> 
> thx
> 
> [...]
> 
Applied, thanks.

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 6/6] avformat/audiointerleave: only keep the retime functionality of the audio interleaver

2020-04-27 Thread Marton Balint




On Sun, 26 Apr 2020, Andreas Rheinhardt wrote:


Marton Balint:

And rename it to retimeinterleave, use the pcm_rechunk bitstream filter for
rechunking.

By seperating the two functions we hopefully get cleaner code.

Signed-off-by: Marton Balint 
---
 configure  |  2 +
 libavformat/Makefile   |  4 +-
 libavformat/gxfenc.c   | 29 
 libavformat/mux.c  |  1 -
 libavformat/mxfenc.c   | 32 ++
 libavformat/retimeinterleave.c | 51 ++
 .../{audiointerleave.h => retimeinterleave.h}  | 31 ++---
 libavformat/utils.c|  1 -
 8 files changed, 111 insertions(+), 40 deletions(-)
 create mode 100644 libavformat/retimeinterleave.c
 rename libavformat/{audiointerleave.h => retimeinterleave.h} (57%)


Why don't you delete audiointerleave.c? (I would have expected to see
more deletions than insertions.)


That was my intention, but it creeped back during a rebase. Will delete 
it.






diff --git a/configure b/configure
index 4f285f0074..f5a84c31bd 100755
--- a/configure
+++ b/configure
@@ -2722,6 +2722,7 @@ fraps_decoder_select="bswapdsp huffman"
 g2m_decoder_deps="zlib"
 g2m_decoder_select="blockdsp idctdsp jpegtables"
 g729_decoder_select="audiodsp"
+gxf_encoder_select="pcm_rechunk_bsf"
 h261_decoder_select="mpegvideo"
 h261_encoder_select="mpegvideoenc"
 h263_decoder_select="h263_parser h263dsp mpegvideo qpeldsp"
@@ -2794,6 +2795,7 @@ mv30_decoder_select="aandcttables blockdsp"
 mvha_decoder_deps="zlib"
 mvha_decoder_select="llviddsp"
 mwsc_decoder_deps="zlib"
+mxf_encoder_select="pcm_rechunk_bsf"
 mxpeg_decoder_select="mjpeg_decoder"
 nellymoser_decoder_select="mdct sinewin"
 nellymoser_encoder_select="audio_frame_queue mdct sinewin"
diff --git a/libavformat/Makefile b/libavformat/Makefile
index d4bed3c113..56ca55fbd5 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -205,7 +205,7 @@ OBJS-$(CONFIG_GIF_DEMUXER)   += gifdec.o
 OBJS-$(CONFIG_GSM_DEMUXER)   += gsmdec.o
 OBJS-$(CONFIG_GSM_MUXER) += rawenc.o
 OBJS-$(CONFIG_GXF_DEMUXER)   += gxf.o
-OBJS-$(CONFIG_GXF_MUXER) += gxfenc.o audiointerleave.o
+OBJS-$(CONFIG_GXF_MUXER) += gxfenc.o retimeinterleave.o
 OBJS-$(CONFIG_G722_DEMUXER)  += g722.o rawdec.o
 OBJS-$(CONFIG_G722_MUXER)+= rawenc.o
 OBJS-$(CONFIG_G723_1_DEMUXER)+= g723_1.o
@@ -347,7 +347,7 @@ OBJS-$(CONFIG_MUSX_DEMUXER)  += musx.o
 OBJS-$(CONFIG_MV_DEMUXER)+= mvdec.o
 OBJS-$(CONFIG_MVI_DEMUXER)   += mvi.o
 OBJS-$(CONFIG_MXF_DEMUXER)   += mxfdec.o mxf.o
-OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o audiointerleave.o 
avc.o
+OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o retimeinterleave.o 
avc.o
 OBJS-$(CONFIG_MXG_DEMUXER)   += mxg.o
 OBJS-$(CONFIG_NC_DEMUXER)+= ncdec.o
 OBJS-$(CONFIG_NISTSPHERE_DEMUXER)+= nistspheredec.o pcm.o
diff --git a/libavformat/gxfenc.c b/libavformat/gxfenc.c
index e7536a6a7e..e95ae99cba 100644
--- a/libavformat/gxfenc.c
+++ b/libavformat/gxfenc.c
@@ -27,7 +27,7 @@
 #include "avformat.h"
 #include "internal.h"
 #include "gxf.h"
-#include "audiointerleave.h"
+#include "retimeinterleave.h"

 #define GXF_AUDIO_PACKET_SIZE 65536

@@ -44,7 +44,7 @@ typedef struct GXFTimecode{
 } GXFTimecode;

 typedef struct GXFStreamContext {
-AudioInterleaveContext aic;
+RetimeInterleaveContext aic;
 uint32_t track_type;
 uint32_t sample_size;
 uint32_t sample_rate;
@@ -813,14 +813,12 @@ static int gxf_write_header(AVFormatContext *s)
 return -1;
 }
 }
+ff_retime_interleave_init(>aic, st->time_base);
 /* FIXME first 10 audio tracks are 0 to 9 next 22 are A to V */
 sc->media_info = media_info<<8 | ('0'+tracks[media_info]++);
 sc->order = s->nb_streams - st->index;
 }

-if (ff_audio_interleave_init(s, GXF_samples_per_frame, (AVRational){ 1, 48000 
}) < 0)
-return -1;
-
 if (tcr && vsc)
 gxf_init_timecode(s, >tc, tcr->value, vsc->fields);

@@ -877,8 +875,6 @@ static void gxf_deinit(AVFormatContext *s)
 {
 GXFContext *gxf = s->priv_data;

-ff_audio_interleave_close(s);
-
 av_freep(>flt_entries);
 av_freep(>map_offsets);
 }
@@ -1016,8 +1012,22 @@ static int gxf_interleave_packet(AVFormatContext *s, 
AVPacket *out, AVPacket *pk
 {
 if (pkt && s->streams[pkt->stream_index]->codecpar->codec_type == 
AVMEDIA_TYPE_VIDEO)
 pkt->duration = 2; // enforce 2 fields
-return ff_audio_rechunk_interleave(s, out, pkt, flush,
-   ff_interleave_packet_per_dts, 
gxf_compare_field_nb);
+return ff_retime_interleave(s, out, pkt, flush,
+

Re: [FFmpeg-devel] [PATCH 5/8] lavc/nvenc: Add hardware config metadata

2020-04-27 Thread Timo Rothenpieler


fixed version applied
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/oggdec: Add page CRC verification

2020-04-27 Thread Michael Niedermayer

On Mon, Apr 27, 2020 at 05:19:49PM +0200, Mattias Wadman wrote:
> Fixes seek issue with ogg files that have segment data that happens to be
> encoded as a "OggS" page syncword. Very unlikely but seems to happen.
> 
> Have been observed to happen with ffmpeg+libvorbis and oggenc encoding
> to 441khz stereo at 160kbps.
> ---
>  libavformat/oggdec.c | 96 +---
>  1 file changed, 73 insertions(+), 23 deletions(-)

breaks chained ogg

./ffmpeg -threads 1 -i ~/tickets/868/chained_streams.ogg -f null -

frame=  954 fps=0.0 q=-0.0 Lsize=N/A time=00:00:31.79 bitrate=N/A speed= 105x
vs
frame=  120 fps=0.0 q=-0.0 Lsize=N/A time=00:00:03.99 bitrate=N/A speed= 289x

stream should be here:
http://v2v.cc/~j/theora_testsuite/chained_streams.ogg

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/8] lavc/nvenc: Add hardware config metadata

2020-04-27 Thread Timo Rothenpieler


On 13.04.2020 17:33, Mark Thompson wrote:

NOT TESTED.



Fails to build:


In file included from libavcodec/nvenc_hevc.c:24:
libavcodec/nvenc.h:220:14: error: unknown type name ‘AVCodecHWConfigInternal’
  220 | extern const AVCodecHWConfigInternal *ff_nvenc_hw_configs[];
  |  ^~~
libavcodec/nvenc_hevc.c:211:23: warning: initialization of ‘const struct 
AVCodecHWConfigInternal **’ from incompatible pointer type ‘const int **’ 
[-Wincompatible-pointer-types]
  211 | .hw_configs = ff_nvenc_hw_configs,
  |   ^~~
libavcodec/nvenc_hevc.c:211:23: note: (near initialization for 
‘ff_hevc_nvenc_encoder.hw_configs’)
In file included from libavcodec/nvenc.c:24:
libavcodec/nvenc.h:220:14: error: unknown type name ‘AVCodecHWConfigInternal’
  220 | extern const AVCodecHWConfigInternal *ff_nvenc_hw_configs[];
  |  ^~~
libavcodec/nvenc.c:59:32: error: conflicting types for ‘ff_nvenc_hw_configs’
   59 | const AVCodecHWConfigInternal *ff_nvenc_hw_configs[] = {
  |^~~
In file included from libavcodec/nvenc.c:24:
libavcodec/nvenc.h:220:39: note: previous declaration of ‘ff_nvenc_hw_configs’ 
was here
  220 | extern const AVCodecHWConfigInternal *ff_nvenc_hw_configs[];
  |   ^~~
CC  libavcodec/parsers.o
In file included from libavcodec/nvenc.c:33:
libavcodec/hwconfig.h:91:28: error: ‘AV_HWDEVICE_TYPE_D3D11’ undeclared here 
(not in a function); did you mean ‘AV_HWDEVICE_TYPE_D3D11VA’?
   91 | .device_type = AV_HWDEVICE_TYPE_ ## device_type_, \
  |^
libavcodec/hwconfig.h:91:28: note: in definition of macro ‘HW_CONFIG_ENCODER’
   91 | .device_type = AV_HWDEVICE_TYPE_ ## device_type_, \
  |^
libavcodec/nvenc.c:63:5: note: in expansion of macro ‘HW_CONFIG_ENCODER_FRAMES’
   63 | HW_CONFIG_ENCODER_FRAMES(D3D11, D3D11),
  | ^~~~
make: *** [ffbuild/common.mak:59: libavcodec/nvenc_hevc.o] Error 1
make: *** Waiting for unfinished jobs
In file included from libavcodec/nvenc_h264.c:24:
libavcodec/nvenc.h:220:14: error: unknown type name ‘AVCodecHWConfigInternal’
  220 | extern const AVCodecHWConfigInternal *ff_nvenc_hw_configs[];
  |  ^~~
libavcodec/nvenc_h264.c:256:23: warning: initialization of ‘const struct 
AVCodecHWConfigInternal **’ from incompatible pointer type ‘const int **’ 
[-Wincompatible-pointer-types]
  256 | .hw_configs = ff_nvenc_hw_configs,
  |   ^~~
libavcodec/nvenc_h264.c:256:23: note: (near initialization for 
‘ff_h264_nvenc_encoder.hw_configs’)
make: *** [ffbuild/common.mak:59: libavcodec/nvenc_h264.o] Error 1
make: *** [ffbuild/common.mak:59: libavcodec/nvenc.o] Error 1



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXT] [PATCH 2/2] avcodec/v4l2_context: Log warning when all capture buffers are in userspace

2020-04-27 Thread Andriy Gelman

Hi Ming,

Thanks for looking over the patch.

On Thu, 09. Apr 07:13, Ming Qian wrote:
> Lgtm,
> 
> But if don't prevent enqueue frame buffer of capture port, unlikely to happen 
> this case.

The problem happens when all the capture packets/frames are buffered internally
by ffmpeg (i.e. the buffer state is V4L2BUF_RET_USER). 

I see it on the s5p-mfc even with your patch: 
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200316020208.27547-1-ming.q...@nxp.com/

> And I think we can get a proper num_frame_buffers by g_ctrl 
> V4L2_CID_MIN_BUFFERS_FOR_CAPTURE and V4L2_CID_MIN_BUFFERS_FOR_OUTPUT.
> 

The values for V4L2_CID_MIN_BUFFERS_FOR_CAPTURE and
V4L2_CID_MIN_BUFFERS_FOR_OUTPUT are the minimum values for the hardware device
and are lower than our defaults. I don't think we should use the minimum values
because the frames/packets may be buffered.

There is another option to request additional buffers via 
CREATE_BUFS ioctl, but it's not supported by all hardware (thanks to ndufresne
on #v4l for suggesting). I feel adding this ioctl should be a separate patch 
and we
still need a warning if it's not supported.

I'll apply this patch on Wednesday if no one objects. 

Thanks,
Andriy

> 
> 
> 
> -Original Message-
> From: ffmpeg-devel  On Behalf Of Andriy 
> Gelman
> Sent: Monday, April 6, 2020 12:22 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Andriy Gelman 
> Subject: [EXT] [FFmpeg-devel] [PATCH 2/2] avcodec/v4l2_context: Log warning 
> when all capture buffers are in userspace
> 
> Caution: EXT Email
> 
> From: Andriy Gelman 
> 
> v4l2_m2m uses device memory mapped buffers to store dequeued frames/packets 
> (reference counted by AVBufferRef). When the reference count drops to zero, 
> the buffer ownership is returned back to the device, so that they can 
> re-filled with frames/packets.
> 
> There are some cases when all the buffers are in userspace (i.e. due to 
> internal buffering in ffmpeg). On the s5p-mfc this causes an infinite wait 
> when polling to dequeue the buffers, which can be prevented by increasing the 
> total number of capture buffers. This commit adds a warning when this occurs.
> 
> Signed-off-by: Andriy Gelman 
> ---
>  libavcodec/v4l2_context.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libavcodec/v4l2_context.c b/libavcodec/v4l2_context.c index 
> 31af10d28e..5145772c45 100644
> --- a/libavcodec/v4l2_context.c
> +++ b/libavcodec/v4l2_context.c
> @@ -285,6 +285,18 @@ static V4L2Buffer* v4l2_dequeue_v4l2buf(V4L2Context 
> *ctx, int timeout)
>  };
>  int i, ret;
> 
> +if (!V4L2_TYPE_IS_OUTPUT(ctx->type) && ctx->buffers) {
> +for (i = 0; i < ctx->num_buffers; i++) {
> +if (ctx->buffers[i].status == V4L2BUF_IN_DRIVER)
> +break;
> +}
> +if (i == ctx->num_buffers)
> +av_log(logger(ctx), AV_LOG_WARNING, "All capture buffers 
> returned to "
> +"userspace. Increase 
> num_capture_buffers "
> +"to prevent device deadlock 
> or dropped "
> +"packets/frames.\n");
> +}
> +
>  /* if we are draining and there are no more capture buffers queued in 
> the driver we are done */
>  if (!V4L2_TYPE_IS_OUTPUT(ctx->type) && ctx_to_m2mctx(ctx)->draining) {
>  for (i = 0; i < ctx->num_buffers; i++) {
> --
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmpeg-develdata=02%7C01%7Cming.qian%40nxp.com%7C38355e6d2f7b41f639f708d7d9e2f2e9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637217441076225890sdata=KmGkJyH%2FxR3jBkrTLI89EWTYhrpNchkJM7ldqjHKUSw%3Dreserved=0
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org 
> with subject "unsubscribe".
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 9/9] lavc/libopenh264enc: Add coder option to replace cabac

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Set DEPRECATED flag to option cabac, replace with coder.

Change the default option to -1 and allow the default cabac to be
determined by profile.

Add FF_API_OPENH264_CABAC macro for cabac to remove this option after
LIBAVCODEC_VERSION_MAJOR = 59.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 35 ---
libavcodec/version.h|  3 +++
2 files changed, 27 insertions(+), 11 deletions(-)


This looks fine

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 8/9] lavc/libopenh264enc: allow specifying the profile through AVCodecContext

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 16 
1 file changed, 16 insertions(+)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 0fe8cf4..4d337d2 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -113,6 +113,22 @@ static av_cold int svc_encode_init_profile(AVCodecContext 
*avctx, SEncParamExt *
{
SVCContext *s = avctx->priv_data;

+/* Allow specifying the libopenh264 profile through AVCodecContext. */
+if (FF_PROFILE_UNKNOWN == s->profile &&
+FF_PROFILE_UNKNOWN != avctx->profile)
+switch (avctx->profile) {
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:
+s->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
+break;
+case FF_PROFILE_H264_HIGH:
+s->profile = FF_PROFILE_H264_HIGH;
+break;
+default:
+av_log(avctx, AV_LOG_WARNING,
+   "Unsupported avctx->profile: %d.\n", avctx->profile);
+break;
+}
+
if (s->profile == FF_PROFILE_UNKNOWN)
s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
FF_PROFILE_H264_CONSTRAINED_BASELINE;


With this in place, why even do the previous commit, why not just go for 
only using avctx->profile? Although as far as I can see, that field 
doesn't seem to have string options for sepcifying H264 profiles, so it 
can only be set via code or by specifying a numberic value - is that 
right?


Wouldn't it just be best to add the H264 profiles to options_table.h to 
keep allowing it to be set via a string, but remove the internal 
s->profile field here, as patch 7/9 already breaks handling of the profile 
field by stopping recognizing "main" and only recognizing "high" anyway.


// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3 5/6] avcodec/pcm_rechunk_bsf: add bitstream filter to rechunk pcm audio

2020-04-27 Thread Marton Balint



On Sun, 26 Apr 2020, Andreas Rheinhardt wrote:


Marton Balint:

Signed-off-by: Marton Balint 
---
 Changelog  |   1 +
 doc/bitstream_filters.texi |  30 ++
 libavcodec/Makefile|   1 +
 libavcodec/bitstream_filters.c |   1 +
 libavcodec/pcm_rechunk_bsf.c   | 206 +
 libavcodec/version.h   |   2 +-
 6 files changed, 240 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/pcm_rechunk_bsf.c

diff --git a/Changelog b/Changelog
index d9fcd8bb0a..6b0c911279 100644
--- a/Changelog
+++ b/Changelog
@@ -59,6 +59,7 @@ version :
 - mv30 decoder
 - Expanded styling support for 3GPP Timed Text Subtitles (movtext)
 - WebP parser
+- pcm_rechunk bitstream filter


 version 4.2:
diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index 8fe5b3ad75..70c276feed 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -548,6 +548,36 @@ ffmpeg -i INPUT -c copy -bsf noise[=1] output.mkv
 @section null
 This bitstream filter passes the packets through unchanged.

+@section pcm_rechunk
+
+Repacketize PCM audio to a fixed number of samples per packet or a fixed packet
+rate per second. This is similar to the @ref{asetnsamples,,asetnsamples audio
+filter,ffmpeg-filters} but works on audio packets instead of audio frames.
+
+@table @option
+@item nb_out_samples, n
+Set the number of samples per each output audio packet. The number is intended
+as the number of samples @emph{per each channel}. Default value is 1024.
+
+@item pad, p
+If set to 1, the filter will pad the last audio packet with silence, so that it
+will contain the same number of samples (or roughly the same number of samples,
+see @option{frame_rate}) as the previous ones. Default value is 1.
+
+@item frame_rate, r
+This option makes the filter output a fixed numer of packets per second instead
+of a fixed number of samples per packet. If the audio sample rate is not
+divisible by the frame rate then the number of samples will not be constant but
+will vary slightly so that each packet will start as close as to the frame


"as close to the frame boundary as possible" or "as close as possible to
the frame boundary"


+boundary as possible. Using this option has precedence over 
@option{nb_out_samples}.
+@end table
+
+You can generate the well known 1602-1601-1602-1601-1602 pattern of 48kHz audio
+for NTSC frame rate using the @option{frame_rate} option.
+@example
+ffmpeg -f lavfi -i sine=r=48000:d=1 -c pcm_s16le -bsf pcm_rechunk=r=3/1001 
-f framecrc -
+@end example
+
 @section prores_metadata

 Modify color property metadata embedded in prores stream.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 88944d9a3a..35968bdaf7 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1115,6 +1115,7 @@ OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += 
mp3_header_decompress_bsf.o \
 OBJS-$(CONFIG_MPEG2_METADATA_BSF) += mpeg2_metadata_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
+OBJS-$(CONFIG_PCM_RECHUNK_BSF)+= pcm_rechunk_bsf.o
 OBJS-$(CONFIG_PRORES_METADATA_BSF)+= prores_metadata_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
 OBJS-$(CONFIG_TEXT2MOVSUB_BSF)+= movsub_bsf.o
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 6b5ffe4d70..9e701191f8 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -49,6 +49,7 @@ extern const AVBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
 extern const AVBitStreamFilter ff_mov2textsub_bsf;
 extern const AVBitStreamFilter ff_noise_bsf;
 extern const AVBitStreamFilter ff_null_bsf;
+extern const AVBitStreamFilter ff_pcm_rechunk_bsf;
 extern const AVBitStreamFilter ff_prores_metadata_bsf;
 extern const AVBitStreamFilter ff_remove_extradata_bsf;
 extern const AVBitStreamFilter ff_text2movsub_bsf;
diff --git a/libavcodec/pcm_rechunk_bsf.c b/libavcodec/pcm_rechunk_bsf.c
new file mode 100644
index 00..2a038fd79b
--- /dev/null
+++ b/libavcodec/pcm_rechunk_bsf.c
@@ -0,0 +1,206 @@
+/*
+ * Copyright (c) 2020 Marton Balint
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include

Re: [FFmpeg-devel] [PATCH v4 7/9] lavc/libopenh264enc: add profile high option support

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Add support for PRO_HIGH/PRO_BASELINE in SVC Encoding extention mode,
which determined by iEntropyCodingModeFlag in ParamTranscode().



Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 32 ++--
1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index a7c389e..0fe8cf4 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -42,7 +42,7 @@ typedef struct SVCContext {
ISVCEncoder *encoder;
int slice_mode;
int loopfilter;
-char *profile;
+int profile;
int max_nal_size;
int skip_frames;
int skipped;
@@ -73,7 +73,11 @@ static const AVOption options[] = {
#endif
#endif
{ "loopfilter", "enable loop filter", OFFSET(loopfilter), AV_OPT_TYPE_INT, 
{ .i64 = 1 }, 0, 1, VE },
-{ "profile", "set profile restrictions", OFFSET(profile), 
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VE },
+{ "profile", "Set profile restrictions", OFFSET(profile), AV_OPT_TYPE_INT, { .i64 = 
FF_PROFILE_UNKNOWN }, FF_PROFILE_UNKNOWN, 0x, VE, "profile" },
+#define PROFILE(name, value)  name, NULL, 0, AV_OPT_TYPE_CONST, { .i64 = value }, 0, 0, 
VE, "profile"
+{ PROFILE("constrained_baseline", 
FF_PROFILE_H264_CONSTRAINED_BASELINE) },
+{ PROFILE("high", FF_PROFILE_H264_HIGH) },
+#undef PROFILE
{ "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), 
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
{ "allow_skip_frames", "allow skipping frames to hit the target bitrate", 
OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
{ "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 
1, VE },
@@ -109,12 +113,28 @@ static av_cold int svc_encode_init_profile(AVCodecContext 
*avctx, SEncParamExt *
{
SVCContext *s = avctx->priv_data;

-if (s->profile && !strcmp(s->profile, "main"))
-param->iEntropyCodingModeFlag = 1; //< 0:CAVLC  1:CABAC
-else if (!s->profile && s->cabac)
+if (s->profile == FF_PROFILE_UNKNOWN)
+s->profile = s->cabac ? FF_PROFILE_H264_HIGH :
+FF_PROFILE_H264_CONSTRAINED_BASELINE;
+
+switch (s->profile) {
+case FF_PROFILE_H264_HIGH:
param->iEntropyCodingModeFlag = 1;
-else
+av_log(avctx, AV_LOG_VERBOSE, "Using CABAC, "
+   "select EProfileIdc PRO_HIGH in libopenh264.\n");
+break;
+case FF_PROFILE_H264_CONSTRAINED_BASELINE:
+case FF_PROFILE_UNKNOWN:
+param->iEntropyCodingModeFlag = 0;
+av_log(avctx, AV_LOG_VERBOSE, "Using CAVLC, "
+   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+break;
+default:
param->iEntropyCodingModeFlag = 0;
+av_log(avctx, AV_LOG_WARNING, "Unsupported profile, "
+   "select EProfileIdc PRO_BASELINE in libopenh264.\n");
+break;
+}

return 0;
}
--
2.7.4


This is fine I guess - apparently it changed in OpenH264 1.8, before that, 
setting iEntropyCodingModeFlag = 1 got main profile, not high.


The commit message feels a bit misleading for what the commit really does 
though. The commit message says "add ... support", which sounds like it 
would be a pure addition, while it is a bit of a rewrite, and actually 
removes support for "-profile main" (even though it might have given 
profile high with current OpenH264 versions).


Maybe this would be clearer:

libopenh264enc: Rewrite profile handling

Support the profiles "constrained_baseline" and "high". Previously, the 
encoder wrapper supported the parameter value "main", but with recent 
versions of OpenH264, the produed bitstream signaled the profile high 
anyway.

---

That's at least clear with what the change actually does.

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 6/9] lavc/libopenh264enc: separate svc_encode_init() into several functions

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Separate the initialization procedure into different functions.

Make it more readable and easier to be extended.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 283 +++-
1 file changed, 174 insertions(+), 109 deletions(-)


I honestly don't see the point here. Yes, the init function is long, but 
you're moving code around and making it even 65 lines longer in total...


In which way is this easier to extend than the previous form?

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 5/9] lavc/libopenh264enc: set slice_mode option to deprecated

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


"slice mode" option seems to be unnecessary since it could be
determined by -slices/max_nal_size.

default:SM_FIXEDSLCNUM_SLICE mode with cpu-number slices.
-slices N:  SM_FIXEDSLCNUM_SLICE mode with N slices.
-max_nal_size:  SM_SIZELIMITED_SLICE mode with limited size slices.

Add FF_API_OPENH264_SLICE_MODE macro to remove this option after
LIBAVCODEC_VERSION_MAJOR = 59.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 7 +--
libavcodec/version.h| 3 +++
2 files changed, 8 insertions(+), 2 deletions(-)


I don't particularly mind this, so ok

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 4/9] lavc/libopenh264enc: prompt slice number changing inside libopenh264

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Libopenh264enc would set the slice according to the number of cpu cores
if uiSliceNum equals to 0 (auto) in SM_FIXEDSLCNUM_SLICE mode.

Prompt a warning for user to catch this.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index f994f95..e556f03 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -237,6 +237,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.sSpatialLayers[0].sSliceCfg.uiSliceMode   = s->slice_mode;
param.sSpatialLayers[0].sSliceCfg.sSliceArgument.uiSliceNum = avctx->slices;
#endif
+if (avctx->slices == 0 && s->slice_mode == SM_FIXEDSLCNUM_SLICE)
+av_log(avctx, AV_LOG_WARNING, "Slice count will be set 
automatically\n");

if (s->slice_mode == SM_SIZELIMITED_SLICE) {
if (s->max_nal_size) {
--
2.7.4


This is ok with me

// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 3/9] lavc/libopenh264enc: add bit rate control select support

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


RC_BITRATE_MODE:
   set BITS_EXCEEDED to iCurrentBitsLevel and allows QP adjust
   in RcCalculatePictureQp().

RC_BUFFERBASED_MODE:
   use buffer status to adjust the video quality.

RC_TIMESTAMP_MODE:
   bit rate control based on timestamp.

Default to use RC_QUALITY_MODE.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index 57313b1..f994f95 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -47,6 +47,9 @@ typedef struct SVCContext {
int skip_frames;
int skipped;
int cabac;
+
+// rate control mode
+int rc_mode;
} SVCContext;

#define OFFSET(x) offsetof(SVCContext, x)
@@ -71,6 +74,13 @@ static const AVOption options[] = {
{ "max_nal_size", "set maximum NAL size in bytes", OFFSET(max_nal_size), 
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE },
{ "allow_skip_frames", "allow skipping frames to hit the target bitrate", 
OFFSET(skip_frames), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
{ "cabac", "Enable cabac", OFFSET(cabac), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 
1, VE },
+
+{ "rc_mode", "Select rate control mode", OFFSET(rc_mode), AV_OPT_TYPE_INT, { .i64 = 
RC_QUALITY_MODE }, RC_OFF_MODE, RC_TIMESTAMP_MODE, VE, "rc_mode" },
+{ "off",   "bit rate control off",   
  0, AV_OPT_TYPE_CONST, { .i64 = RC_OFF_MODE }, 0, 0, VE, "rc_mode" },
+{ "quality",   "quality mode",   
  0, AV_OPT_TYPE_CONST, { .i64 = RC_QUALITY_MODE }, 0, 0, VE, "rc_mode" },
+{ "bitrate",   "bitrate mode",   
  0, AV_OPT_TYPE_CONST, { .i64 = RC_BITRATE_MODE }, 0, 0, VE, "rc_mode" },
+{ "buffer","using buffer status to adjust the video quality (no bitrate 
control)", 0, AV_OPT_TYPE_CONST, { .i64 = RC_BUFFERBASED_MODE }, 0, 0, VE, "rc_mode" },
+{ "timestamp", "bit rate control based on timestamp",
  0, AV_OPT_TYPE_CONST, { .i64 = RC_TIMESTAMP_MODE },   0, 0, VE, "rc_mode" },
{ NULL }
};

@@ -134,7 +144,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.iPicHeight = avctx->height;
param.iTargetBitrate = avctx->bit_rate > 0 ? avctx->bit_rate : 
2*1000*1000;
param.iMaxBitrate= FFMAX(avctx->rc_max_rate, 
avctx->bit_rate);
-param.iRCMode= RC_QUALITY_MODE;
+param.iRCMode= s->rc_mode;
// QP = 0 is not well supported, so default to (1, 51)
param.iMaxQp = avctx->qmax >= 0 ? av_clip(avctx->qmax, 
1, 51) : 51;
param.iMinQp = avctx->qmin >= 0 ? av_clip(avctx->qmin, 
1, param.iMaxQp) : 1;
--
2.7.4


This looks ok, but I don't think all of these have been available since 
the beginning. We do support building with a few older versions of the 
library, so I think at least RC_TIMESTAMP_MODE appeared later. The lowest 
version that was supported originally was OpenH264 1.3, so for things that 
have appeared later, please add ifdefs (or consistently bump the minimum 
version somewhere and remove redundant checks for lower versions).


// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 2/9] lavc/libopenh264enc: add default gop size and bit rate

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


It would be 200kbps bitrate with gop size = 12 by default
which generated too many IDR frames in rather low bit rate.
The quality would be poor.

Set these default values according to vaapi encoder, and
use 2Mbps bitrate if user doesn't set it explicitly as
nvenc sugguested.

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index c7ae5b1..57313b1 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -132,7 +132,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.fMaxFrameRate  = 1/av_q2d(avctx->time_base);
param.iPicWidth  = avctx->width;
param.iPicHeight = avctx->height;
-param.iTargetBitrate = avctx->bit_rate;
+param.iTargetBitrate = avctx->bit_rate > 0 ? avctx->bit_rate : 
2*1000*1000;
param.iMaxBitrate= FFMAX(avctx->rc_max_rate, 
avctx->bit_rate);
param.iRCMode= RC_QUALITY_MODE;
// QP = 0 is not well supported, so default to (1, 51)
@@ -335,6 +335,8 @@ static int svc_encode_frame(AVCodecContext *avctx, AVPacket 
*avpkt,
}

static const AVCodecDefault svc_enc_defaults[] = {
+{ "b", "0" },
+{ "g", "120"   },
{ "qmin",  "-1"},


Why do you hardcode a value for g here, but put the default bitrate value 
in the code above? Wouldn't it be clearer to have both defaults here at 
the same place?


// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v4 1/9] lavc/libopenh264enc: Add default qmin/qmax support

2020-04-27 Thread Martin Storsjö


On Wed, 15 Apr 2020, Linjie Fu wrote:


Set default QP range to (1, 51) instead of (2, 32).

QP = 0 is not well supported currently in libopenh264. If iMaxQp/iMinQp
equals 0, the QP range would be changed unexpectedly inside libopenh264
with a warning:

Warning:Change QP Range from(0,51) to (12,42)

[1] 

[2] 

Signed-off-by: Linjie Fu 
---
libavcodec/libopenh264enc.c | 10 ++
1 file changed, 10 insertions(+)

diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
index dd5d4ee..c7ae5b1 100644
--- a/libavcodec/libopenh264enc.c
+++ b/libavcodec/libopenh264enc.c
@@ -135,6 +135,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
param.iTargetBitrate = avctx->bit_rate;
param.iMaxBitrate= FFMAX(avctx->rc_max_rate, 
avctx->bit_rate);
param.iRCMode= RC_QUALITY_MODE;
+// QP = 0 is not well supported, so default to (1, 51)
+param.iMaxQp = avctx->qmax >= 0 ? av_clip(avctx->qmax, 
1, 51) : 51;
+param.iMinQp = avctx->qmin >= 0 ? av_clip(avctx->qmin, 
1, param.iMaxQp) : 1;


If qmax/qmin < 0, i.e. wasn't specified by the user, wouldn't it be better 
to not touch param.iMax/MinQp at all (and use the default value of the 
library, which may change between versions), instead of overriding it with 
a value hardcoded here?


// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v10 2/2] avformat: add demuxer for Pro Pinball Series' Soundbanks

2020-04-27 Thread Michael Niedermayer

On Sun, Apr 26, 2020 at 11:49:00PM +, Zane van Iperen wrote:
> On Sun, 26 Apr 2020 00:07:52 +0200
> "Michael Niedermayer"  wrote:
> 
> > 
> > also iam not sure it makes sense or if it goes to far but the probe
> > code could check all tracks which are within the buffer
> > all these low 10 scores are a bit ugly, some more solid "yes iam sure
> > this is / is not" instead of such really low scores.
> > 
> > low scores which get returned based on few weak checks have the
> > problem of more often producing errors in detection. Random non
> > multimedia files being detected as something they are not or if 2
> > demuxers have such a low score detection then its more likely the
> > wrong one will be choosen
> > 
> > 
> 
> How's this?
> 
> I've removed the "track_count > 113" check and made it
> all-or-nothing.
> 
> static int pp_bnk_probe(const AVProbeData *p)
> {
> uint32_t sample_rate = AV_RL32(p->buf +  4);
> uint32_t track_count = AV_RL32(p->buf + 12);
> uint32_t flags   = AV_RL32(p->buf + 16);
> 
> if (track_count == 0 || track_count > INT_MAX)
> return 0;
> 
> if ((sample_rate !=  5512) && (sample_rate != 11025) &&
> (sample_rate != 22050) && (sample_rate != 44100))
> return 0;
> 
> /* Check the first track header. */
> if (AV_RL32(p->buf + 28) != sample_rate)
> return 0;
> 
> if ((flags & ~PP_BNK_FLAG_MASK) != 0)
> return 0;
> 
> return AVPROBE_SCORE_MAX / 4 + 1;
> }

ok until someone sees a way to improve this

thx


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] swscale/vscale: Increase type strictness

2020-04-27 Thread Michael Niedermayer

On Sun, Apr 26, 2020 at 09:49:57AM +0200, Andreas Rheinhardt wrote:
> libswscale/vscale.c makes extensive use of function pointers and in
> doing so it converts these function pointers to and from a pointer to
> void. Yet this is actually against the C standard:
> C90 only guarantees that one can convert a pointer to any incomplete
> type or object type to void* and back with the result comparing equal
> to the original which makes pointers to void generic pointers to
> incomplete or object type. Yet C90 lacks a generic function pointer
> type.
> C99 additionally guarantees that a pointer to a function of one type may
> be converted to a pointer to a function of another type and when
> converting back with the original and the result comparing equal.
> This makes any function pointer type a generic function pointer type.
> Yet even this does not make pointers to void generic function pointers.
> 
> Both GCC and Clang emit warnings for this when in pedantic mode.
> 
> This commit fixes this by using a union that can hold one member of any
> of the required function pointer types to store the function pointer.
> This works even for C90.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libswscale/vscale.c | 51 ++---
>  1 file changed, 29 insertions(+), 22 deletions(-)

patch ok

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avfilter: add asubboost filter

2020-04-27 Thread Paul B Mahol

Signed-off-by: Paul B Mahol 
---
 doc/filters.texi   |  31 ++
 libavfilter/Makefile   |   1 +
 libavfilter/af_asubboost.c | 210 +
 libavfilter/allfilters.c   |   1 +
 4 files changed, 243 insertions(+)
 create mode 100644 libavfilter/af_asubboost.c

diff --git a/doc/filters.texi b/doc/filters.texi
index 71a6787289..18c8528da7 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -2446,6 +2446,37 @@ Number of points where the waveform crosses the zero 
level axis.
 Rate of Zero crossings and number of audio samples.
 @end table
 
+@section asubboost
+Boost subwoofer frequencies.
+
+The filter accepts the following options:
+
+@table @option
+@item dry
+Set dry gain, how much of original signal is kept. Allowed range is from 0 to 
1.
+Default value is 0.5.
+
+@item wet
+Set wet gain, how much of filtered signal is kept. Allowed range is from 0 to 
1.
+Default value is 0.8.
+
+@item decay
+Set delay line decay gain value. Allowed range is from 0 to 1.
+Default value is 0.7.
+
+@item feedback
+Set delay line feedback gain value. Allowed range is from 0 to 1.
+Default value is 0.5.
+
+@item cutoff
+Set cutoff frequenciy in herz. Allowed range is 50 to 250.
+Default value is 100.
+
+@item delay
+Set delay. Allowed range is from 1 to 100.
+Default value is 20.
+@end table
+
 @section atempo
 
 Adjust audio tempo.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index e1205eb063..f982afe15f 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -86,6 +86,7 @@ OBJS-$(CONFIG_ASPLIT_FILTER) += split.o
 OBJS-$(CONFIG_ASR_FILTER)+= af_asr.o
 OBJS-$(CONFIG_ASTATS_FILTER) += af_astats.o
 OBJS-$(CONFIG_ASTREAMSELECT_FILTER)  += f_streamselect.o framesync.o
+OBJS-$(CONFIG_ASUBBOOST_FILTER)  += af_asubboost.o
 OBJS-$(CONFIG_ATEMPO_FILTER) += af_atempo.o
 OBJS-$(CONFIG_ATRIM_FILTER)  += trim.o
 OBJS-$(CONFIG_AXCORRELATE_FILTER)+= af_axcorrelate.o
diff --git a/libavfilter/af_asubboost.c b/libavfilter/af_asubboost.c
new file mode 100644
index 00..37ed853687
--- /dev/null
+++ b/libavfilter/af_asubboost.c
@@ -0,0 +1,210 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/channel_layout.h"
+#include "libavutil/ffmath.h"
+#include "libavutil/opt.h"
+#include "avfilter.h"
+#include "audio.h"
+#include "formats.h"
+
+typedef struct ASubBoostContext {
+const AVClass *class;
+
+double dry_gain;
+double wet_gain;
+double feedback;
+double decay;
+double delay;
+double cutoff;
+
+double a0, a1, a2;
+double b0, b1, b2;
+
+int write_pos;
+int buffer_samples;
+
+AVFrame *i, *o;
+AVFrame *buffer;
+} ASubBoostContext;
+
+static int query_formats(AVFilterContext *ctx)
+{
+AVFilterFormats *formats = NULL;
+AVFilterChannelLayouts *layouts = NULL;
+static const enum AVSampleFormat sample_fmts[] = {
+AV_SAMPLE_FMT_DBLP,
+AV_SAMPLE_FMT_NONE
+};
+int ret;
+
+formats = ff_make_format_list(sample_fmts);
+if (!formats)
+return AVERROR(ENOMEM);
+ret = ff_set_common_formats(ctx, formats);
+if (ret < 0)
+return ret;
+
+layouts = ff_all_channel_counts();
+if (!layouts)
+return AVERROR(ENOMEM);
+
+ret = ff_set_common_channel_layouts(ctx, layouts);
+if (ret < 0)
+return ret;
+
+formats = ff_all_samplerates();
+return ff_set_common_samplerates(ctx, formats);
+}
+
+static int config_input(AVFilterLink *inlink)
+{
+AVFilterContext *ctx = inlink->dst;
+ASubBoostContext *s = ctx->priv;
+
+double w0 = 2 * M_PI * s->cutoff / inlink->sample_rate;
+double alpha = sin(w0) / 2 * sqrt(2 * (1 / 0.5 - 1) + 2);
+
+s->a0 =  1 + alpha;
+s->a1 = -2 * cos(w0);
+s->a2 =  1 - alpha;
+s->b0 = (1 - cos(w0)) / 2;
+s->b1 =  1 - cos(w0);
+s->b2 = (1 - cos(w0)) / 2;
+
+s->a1 /= s->a0;
+s->a2 /= s->a0;
+s->b0 /= s->a0;
+s->b1 /= s->a0;
+s->b2 /= s->a0;
+
+s->buffer_samples = inlink->sample_rate * s->delay / 1000;
+
+s->buffer = ff_get_audio_buffer(inlink, s->buffer_samples);
+s->i = ff_get_audio_buffer(inlink, 2);
+s->o =

Re: [FFmpeg-devel] [PATCH, v2 1/2] lavc/qsvdec: add decode support for HEVC 4:2:2 8-bit and 10-bit

2020-04-27 Thread Artem Galin

On Wed, 15 Apr 2020 at 05:02, Fu, Linjie  wrote:

> > From: Zhong Li 
> > Sent: Wednesday, April 15, 2020 09:58
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Cc: Fu, Linjie 
> > Subject: Re: [FFmpeg-devel] [PATCH, v2 1/2] lavc/qsvdec: add decode
> > support for HEVC 4:2:2 8-bit and 10-bit
> >
> > Linjie Fu  于2020年2月26日周三 下午4:43写道：
> > >
> > > Enables HEVC Range Extension decoding support (Linux) for 4:2:2 8/10
> bit
> > > on ICL+ (gen11 +) platform.
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > > [v2]: restrict to support on Linux.
> > >
> > >  libavcodec/qsv.c  | 16 
> > >  libavutil/hwcontext_qsv.c | 24 
> > >  2 files changed, 40 insertions(+)
> > >
> > > diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
> > > index db98c75..171a8d6 100644
> > > --- a/libavcodec/qsv.c
> > > +++ b/libavcodec/qsv.c
> > > @@ -195,6 +195,12 @@ enum AVPixelFormat ff_qsv_map_fourcc(uint32_t
> > fourcc)
> > >  case MFX_FOURCC_NV12: return AV_PIX_FMT_NV12;
> > >  case MFX_FOURCC_P010: return AV_PIX_FMT_P010;
> > >  case MFX_FOURCC_P8:   return AV_PIX_FMT_PAL8;
> > > +#if CONFIG_VAAPI
>
LGTM. CONFIG_VAAPI is not needed here because crash does not related to
these changes.
Full support MFX_FOURCC_YUY2 is WIP for Windows.

> > > +case MFX_FOURCC_YUY2: return AV_PIX_FMT_YUYV422;
> >
> > There is no VAAPI structures here, so should not use CONFIG_VAAPI to
> > disable them.
> >
>
> This is pointed out in [1] that D3D code doesn't support YUYV format, and
> indeed
> It leads to unexpected crash in windows.(instead of working or reporting
> unsupported
> On ICL- platform)
>
> Hence this patch restricted to add support on linux only.
>
> And I admit the best solution should be get this fully supported on both
> linux and windows.
> (I believe Max and Artem is helping on windows side)
>
> Thanks for the review,
> Linjie
>
> [1]
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1582596080-1035-1-git-send-email-linjie...@intel.com/
>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/oggdec: Add page CRC verification

2020-04-27 Thread Mattias Wadman

Fixes seek issue with ogg files that have segment data that happens to be
encoded as a "OggS" page syncword. Very unlikely but seems to happen.

Have been observed to happen with ffmpeg+libvorbis and oggenc encoding
to 441khz stereo at 160kbps.
---
 libavformat/oggdec.c | 96 +---
 1 file changed, 73 insertions(+), 23 deletions(-)

diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index 95190589ab..22532f0341 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -31,6 +31,8 @@
 #include 
 #include "libavutil/avassert.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/crc.h"
+#include "libavcodec/bytestream.h"
 #include "oggdec.h"
 #include "avformat.h"
 #include "internal.h"
@@ -339,14 +341,24 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
 AVIOContext *bc = s->pb;
 struct ogg *ogg = s->priv_data;
 struct ogg_stream *os;
-int ret, i = 0;
-int flags, nsegs;
+int ret, i;
+int version, flags, nsegs;
 uint64_t gp;
 uint32_t serial;
+uint32_t crc;
 int size, idx;
 uint8_t sync[4];
-int sp = 0;
-
+uint8_t header[23];
+GetByteContext headergb;
+uint8_t segments[255];
+uint8_t *segmentsdata;
+int sp;
+const AVCRC *crc_table;
+uint32_t ccrc;
+
+again:
+sp = 0;
+i = 0;
 ret = avio_read(bc, sync, 4);
 if (ret < 4)
 return ret < 0 ? ret : AVERROR_EOF;
@@ -378,19 +390,59 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
 return AVERROR_INVALIDDATA;
 }
 
-if (avio_r8(bc) != 0) {  /* version */
-av_log (s, AV_LOG_ERROR, "ogg page, unsupported version\n");
+ret = avio_read(bc, header, sizeof(header));
+if (ret < sizeof(header))
+return AVERROR_INVALIDDATA;
+nsegs = header[22];
+ret = avio_read(bc, segments, nsegs);
+if (ret < nsegs)
+return AVERROR_INVALIDDATA;
+size = 0;
+for (i = 0; i < nsegs; i++)
+size += segments[i];
+
+bytestream2_init(, header, sizeof(header));
+version = bytestream2_get_byte();
+flags   = bytestream2_get_byte();
+gp  = bytestream2_get_le64();
+serial  = bytestream2_get_le32();
+bytestream2_skip(, 4); // seq le32
+crc = bytestream2_get_le32();
+
+segmentsdata = av_malloc(size);
+if (!segmentsdata)
+return AVERROR(ENOMEM);
+ret = avio_read(bc, segmentsdata, size);
+if (ret < size) {
+av_freep();
 return AVERROR_INVALIDDATA;
 }
 
-flags  = avio_r8(bc);
-gp = avio_rl64(bc);
-serial = avio_rl32(bc);
-avio_skip(bc, 8); /* seq, crc */
-nsegs  = avio_r8(bc);
+// Reset CRC in header to zero and calculate for whole page
+crc_table = av_crc_get_table(AV_CRC_32_IEEE);
+memset([18], 0, 4);
+ccrc = 0;
+ccrc = av_crc(crc_table, ccrc, "OggS", 4);
+ccrc = av_crc(crc_table, ccrc, header, sizeof(header));
+ccrc = av_crc(crc_table, ccrc, segments, nsegs);
+ccrc = av_crc(crc_table, ccrc, segmentsdata, size);
+// Default AV_CRC_32_IEEE table is BE
+ccrc = av_bswap32(ccrc);
+
+if (ccrc != crc) {
+av_log(s, AV_LOG_TRACE, "ogg page, invalid checksum %x != %x\n", ccrc, 
crc);
+if (s->error_recognition & AV_EF_CRCCHECK) {
+av_freep();
+// TODO: smarter use of read data?
+goto again;
+}
+}
 
-if (avio_feof(bc))
-return AVERROR_EOF;
+if (version != 0) {
+av_log(s, AV_LOG_ERROR, "ogg page, unsupported version %d\n", version);
+av_freep();
+return AVERROR_INVALIDDATA;
+}
 
 idx = ogg_find_stream(ogg, serial);
 if (idx < 0) {
@@ -401,24 +453,24 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
 
 if (idx < 0) {
 av_log(s, AV_LOG_ERROR, "failed to create or replace stream\n");
+av_freep();
 return idx;
 }
 }
 
 os = ogg->streams + idx;
 ogg->page_pos =
-os->page_pos = avio_tell(bc) - 27;
+os->page_pos = avio_tell(bc) - 27 - size - nsegs;;
 
 if (os->psize > 0) {
 ret = ogg_new_buf(ogg, idx);
-if (ret < 0)
+if (ret < 0) {
+av_freep();
 return ret;
+}
 }
 
-ret = avio_read(bc, os->segments, nsegs);
-if (ret < nsegs)
-return ret < 0 ? ret : AVERROR_EOF;
-
+memcpy(os->segments, segments, nsegs);
 os->nsegs = nsegs;
 os->segp  = 0;
 
@@ -456,10 +508,8 @@ static int ogg_read_page(AVFormatContext *s, int *sid)
 os->buf = nb;
 }
 
-ret = avio_read(bc, os->buf + os->bufpos, size);
-if (ret < size)
-return ret < 0 ? ret : AVERROR_EOF;
-
+memcpy(os->buf + os->bufpos, segmentsdata, size);
+av_freep();
 os->bufpos += size;
 os->granule = gp;
 os->flags   = flags;
-- 
2.18.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH] libavutil: add clean aperture (CLAP) side data.

2020-04-27 Thread Lynne

Apr 27, 2020, 05:27 by neil.birkb...@gmail.com:

> The clean aperature represents a cropping of the stored image data used to
> relate the image data to a canonical video system and exists as container
> metadata (see 'clap' section in 
> https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html)
>
> Addition of the side data is a first step towards demuxing CLAP atom metadata,
> helping to resolve https://trac.ffmpeg.org/ticket/7437
>
> This CleanAperture representation can also carry PixelCrop fields from MKV.
> Side data was suggested as a way to carry such PixelCrop fields in:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2016-March/192302.html
>
> Transmuxing the side data can then be added (MOV to/from MKV), and
> auto-application could optionally be enabled like autorotate in 
> ffmpeg_filter.c.
>

We already have frame cropping metadata in the AVFrame structure.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> Command line?

There is none.

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:35，Nicolas George  写道：
> 
> Steven Liu (12020-04-27):
>> I need one example to understand about the security issue after this patch.
> 
> Use ff_make_absolute_url() on a trusted base and an un-trusted path;
> check the result starts with the allowed prefix. Let an attacker escape
> because the result contains ../.
> 
Command line?
> Regards,
> 
> -- 
>  Nicolas George

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> I need one example to understand about the security issue after this patch.

Use ff_make_absolute_url() on a trusted base and an un-trusted path;
check the result starts with the allowed prefix. Let an attacker escape
because the result contains ../.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:22，Nicolas George  写道：
> 
> Steven Liu (12020-04-27):
>> /../../../../../other/url,  this is the absolute path, so just concat and 
>> don’t process,
>> Or what do you want to say?
> 
> This is not an absolute path, since it contains "..". I think it is a
> problem that the output of ff_make_absolute_url() is not, you know,
> absolute.
> 
> It can even be considered a security issue, since other parts of the
> code could assume that the output of ff_make_absolute_url() is actually
> absolute.
I need one example to understand about the security issue after this patch.


Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> /../../../../../other/url,  this is the absolute path, so just concat and 
> don’t process,
> Or what do you want to say?

This is not an absolute path, since it contains "..". I think it is a
problem that the output of ff_make_absolute_url() is not, you know,
absolute.

It can even be considered a security issue, since other parts of the
code could assume that the output of ff_make_absolute_url() is actually
absolute.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:14，Nicolas George  写道：
> 
> Steven Liu (12020-04-27):
>> fix ticket: 8625
>> and add testcase into url for double dot corner case
>> 
>> Signed-off-by: Steven Liu 
>> ---
>> libavformat/tests/url.c |  3 +++
>> libavformat/url.c   | 21 +++--
>> tests/ref/fate/url  |  3 +++
>> 3 files changed, 25 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
>> index 5e484fd428..02d0d59aa8 100644
>> --- a/libavformat/tests/url.c
>> +++ b/libavformat/tests/url.c
>> @@ -56,6 +56,7 @@ int main(void)
>> test("/foo/bar", "baz");
>> test("/foo/bar", "../baz");
>> test("/foo/bar", "/baz");
>> +test("/foo/bar", "../../../baz");
>> test("http://server/foo/;, "baz");
>> test("http://server/foo/bar;, "baz");
>> test("http://server/foo/;, "../baz");
>> @@ -65,6 +66,8 @@ int main(void)
>> test("http://server/foo/bar?param=value/with/slashes;, "/baz");
>> test("http://server/foo/bar?param;, "?someparam");
>> test("http://server/foo/bar;, "//other/url");
>> +test("http://server/foo/bar;, "../../../../../other/url");
>> +test("http://server/foo/bar;, "/../../../../../other/url");
>> 
>> printf("\nTesting av_url_split:\n");
>> test2("/foo/bar");
>> diff --git a/libavformat/url.c b/libavformat/url.c
>> index 596fb49cfc..0aa50ab9a7 100644
>> --- a/libavformat/url.c
>> +++ b/libavformat/url.c
>> @@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
>> *base,
>>   const char *rel)
>> {
>> char *sep, *path_query;
>> +char *root, *p;
>> /* Absolute path, relative to the current server */
>> if (base && strstr(base, "://") && rel[0] == '/') {
>> if (base != buf)
>> @@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const 
>> char *base,
>> return;
>> }
>> 
>> +root = p = buf;
>> +/* Get the path root of the url which start by "://" */
>> +if (p && strstr(p, "://")) {
>> +sep = strstr(p, "://");
>> +if (sep) {
>> +sep += 3;
>> +root = strchr(sep, '/');
>> +}
>> +}
>> +
>> /* Remove the file name from the base url */
>> sep = strrchr(buf, '/');
>> +if (sep <= root)
>> +sep = root;
>> +
>> if (sep)
>> sep[1] = '\0';
>> else
>> buf[0] = '\0';
>> while (av_strstart(rel, "../", NULL) && sep) {
>> /* Remove the path delimiter at the end */
>> -sep[0] = '\0';
>> -sep = strrchr(buf, '/');
>> +if (sep > root) {
>> +sep[0] = '\0';
>> +sep = strrchr(buf, '/');
>> +}
>> +
>> /* If the next directory name to pop off is "..", break here */
>> if (!strcmp(sep ? [1] : buf, "..")) {
>> /* Readd the slash we just removed */
>> diff --git a/tests/ref/fate/url b/tests/ref/fate/url
>> index 980b2ce1f9..c8260a97be 100644
>> --- a/tests/ref/fate/url
>> +++ b/tests/ref/fate/url
>> @@ -3,6 +3,7 @@ Testing ff_make_absolute_url:
>>   /foo/bar baz  => 
>> /foo/baz
>>   /foo/bar ../baz   => 
>> /baz
>>   /foo/bar /baz => 
>> /baz
>> +  /foo/bar ../../../baz => 
>> /baz
>> http://server/foo/ baz  => 
>> http://server/foo/baz
>>  http://server/foo/bar baz  => 
>> http://server/foo/baz
>> http://server/foo/ ../baz   => 
>> http://server/baz
>> @@ -12,6 +13,8 @@ Testing ff_make_absolute_url:
>> http://server/foo/bar?param=value/with/slashes /baz => 
>> http://server/baz
>> http://server/foo/bar?param ?someparam   => 
>> http://server/foo/bar?someparam
>>  http://server/foo/bar //other/url  => 
>> http://other/url
>> + http://server/foo/bar ../../../../../other/url 
>> => http://server/other/url
> 
>> + http://server/foo/bar 
>> /../../../../../other/url => http://server/../../../../../other/url
> 
> Is this supposed to be the proper result?
/../../../../../other/url,  this is the absolute path, so just concat and don’t 
process,
Or what do you want to say?
> 
>> 
>> Testing av_url_split:
>> /foo/bar =>  
>>   -1 /foo/bar
> 
> Regards,
> 
> -- 
>  Nicolas George

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> fix ticket: 8625
> and add testcase into url for double dot corner case
> 
> Signed-off-by: Steven Liu 
> ---
>  libavformat/tests/url.c |  3 +++
>  libavformat/url.c   | 21 +++--
>  tests/ref/fate/url  |  3 +++
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
> index 5e484fd428..02d0d59aa8 100644
> --- a/libavformat/tests/url.c
> +++ b/libavformat/tests/url.c
> @@ -56,6 +56,7 @@ int main(void)
>  test("/foo/bar", "baz");
>  test("/foo/bar", "../baz");
>  test("/foo/bar", "/baz");
> +test("/foo/bar", "../../../baz");
>  test("http://server/foo/;, "baz");
>  test("http://server/foo/bar;, "baz");
>  test("http://server/foo/;, "../baz");
> @@ -65,6 +66,8 @@ int main(void)
>  test("http://server/foo/bar?param=value/with/slashes;, "/baz");
>  test("http://server/foo/bar?param;, "?someparam");
>  test("http://server/foo/bar;, "//other/url");
> +test("http://server/foo/bar;, "../../../../../other/url");
> +test("http://server/foo/bar;, "/../../../../../other/url");
>  
>  printf("\nTesting av_url_split:\n");
>  test2("/foo/bar");
> diff --git a/libavformat/url.c b/libavformat/url.c
> index 596fb49cfc..0aa50ab9a7 100644
> --- a/libavformat/url.c
> +++ b/libavformat/url.c
> @@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
> *base,
>const char *rel)
>  {
>  char *sep, *path_query;
> +char *root, *p;
>  /* Absolute path, relative to the current server */
>  if (base && strstr(base, "://") && rel[0] == '/') {
>  if (base != buf)
> @@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const 
> char *base,
>  return;
>  }
>  
> +root = p = buf;
> +/* Get the path root of the url which start by "://" */
> +if (p && strstr(p, "://")) {
> +sep = strstr(p, "://");
> +if (sep) {
> +sep += 3;
> +root = strchr(sep, '/');
> +}
> +}
> +
>  /* Remove the file name from the base url */
>  sep = strrchr(buf, '/');
> +if (sep <= root)
> +sep = root;
> +
>  if (sep)
>  sep[1] = '\0';
>  else
>  buf[0] = '\0';
>  while (av_strstart(rel, "../", NULL) && sep) {
>  /* Remove the path delimiter at the end */
> -sep[0] = '\0';
> -sep = strrchr(buf, '/');
> +if (sep > root) {
> +sep[0] = '\0';
> +sep = strrchr(buf, '/');
> +}
> +
>  /* If the next directory name to pop off is "..", break here */
>  if (!strcmp(sep ? [1] : buf, "..")) {
>  /* Readd the slash we just removed */
> diff --git a/tests/ref/fate/url b/tests/ref/fate/url
> index 980b2ce1f9..c8260a97be 100644
> --- a/tests/ref/fate/url
> +++ b/tests/ref/fate/url
> @@ -3,6 +3,7 @@ Testing ff_make_absolute_url:
>/foo/bar baz  => 
> /foo/baz
>/foo/bar ../baz   => 
> /baz
>/foo/bar /baz => 
> /baz
> +  /foo/bar ../../../baz => 
> /baz
>  http://server/foo/ baz  => 
> http://server/foo/baz
>   http://server/foo/bar baz  => 
> http://server/foo/baz
>  http://server/foo/ ../baz   => 
> http://server/baz
> @@ -12,6 +13,8 @@ Testing ff_make_absolute_url:
>  http://server/foo/bar?param=value/with/slashes /baz => 
> http://server/baz
>  http://server/foo/bar?param ?someparam   => 
> http://server/foo/bar?someparam
>   http://server/foo/bar //other/url  => 
> http://other/url
> + http://server/foo/bar ../../../../../other/url 
> => http://server/other/url

> + http://server/foo/bar /../../../../../other/url 
> => http://server/../../../../../other/url

Is this supposed to be the proper result?

>  
>  Testing av_url_split:
>  /foo/bar =>  
>   -1 /foo/bar

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:10，Nicolas George  写道：
> 
> Steven Liu (12020-04-27):
>> Do you mean you don’t want in the cc list?
>> I think can remove you from cc list if you don’t want in it.
> 
> I want you to do exactly as the headers direct you to do.
Inner peace.



Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu

fix ticket: 8625
and add testcase into url for double dot corner case

Signed-off-by: Steven Liu 
---
 libavformat/tests/url.c |  3 +++
 libavformat/url.c   | 21 +++--
 tests/ref/fate/url  |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
index 5e484fd428..02d0d59aa8 100644
--- a/libavformat/tests/url.c
+++ b/libavformat/tests/url.c
@@ -56,6 +56,7 @@ int main(void)
 test("/foo/bar", "baz");
 test("/foo/bar", "../baz");
 test("/foo/bar", "/baz");
+test("/foo/bar", "../../../baz");
 test("http://server/foo/;, "baz");
 test("http://server/foo/bar;, "baz");
 test("http://server/foo/;, "../baz");
@@ -65,6 +66,8 @@ int main(void)
 test("http://server/foo/bar?param=value/with/slashes;, "/baz");
 test("http://server/foo/bar?param;, "?someparam");
 test("http://server/foo/bar;, "//other/url");
+test("http://server/foo/bar;, "../../../../../other/url");
+test("http://server/foo/bar;, "/../../../../../other/url");
 
 printf("\nTesting av_url_split:\n");
 test2("/foo/bar");
diff --git a/libavformat/url.c b/libavformat/url.c
index 596fb49cfc..0aa50ab9a7 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
   const char *rel)
 {
 char *sep, *path_query;
+char *root, *p;
 /* Absolute path, relative to the current server */
 if (base && strstr(base, "://") && rel[0] == '/') {
 if (base != buf)
@@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
 return;
 }
 
+root = p = buf;
+/* Get the path root of the url which start by "://" */
+if (p && strstr(p, "://")) {
+sep = strstr(p, "://");
+if (sep) {
+sep += 3;
+root = strchr(sep, '/');
+}
+}
+
 /* Remove the file name from the base url */
 sep = strrchr(buf, '/');
+if (sep <= root)
+sep = root;
+
 if (sep)
 sep[1] = '\0';
 else
 buf[0] = '\0';
 while (av_strstart(rel, "../", NULL) && sep) {
 /* Remove the path delimiter at the end */
-sep[0] = '\0';
-sep = strrchr(buf, '/');
+if (sep > root) {
+sep[0] = '\0';
+sep = strrchr(buf, '/');
+}
+
 /* If the next directory name to pop off is "..", break here */
 if (!strcmp(sep ? [1] : buf, "..")) {
 /* Readd the slash we just removed */
diff --git a/tests/ref/fate/url b/tests/ref/fate/url
index 980b2ce1f9..c8260a97be 100644
--- a/tests/ref/fate/url
+++ b/tests/ref/fate/url
@@ -3,6 +3,7 @@ Testing ff_make_absolute_url:
   /foo/bar baz  => 
/foo/baz
   /foo/bar ../baz   => /baz
   /foo/bar /baz => /baz
+  /foo/bar ../../../baz => /baz
 http://server/foo/ baz  => 
http://server/foo/baz
  http://server/foo/bar baz  => 
http://server/foo/baz
 http://server/foo/ ../baz   => 
http://server/baz
@@ -12,6 +13,8 @@ Testing ff_make_absolute_url:
 http://server/foo/bar?param=value/with/slashes /baz => 
http://server/baz
 http://server/foo/bar?param ?someparam   => 
http://server/foo/bar?someparam
  http://server/foo/bar //other/url  => 
http://other/url
+ http://server/foo/bar ../../../../../other/url => 
http://server/other/url
+ http://server/foo/bar /../../../../../other/url 
=> http://server/../../../../../other/url
 
 Testing av_url_split:
 /foo/bar =>
-1 /foo/bar
-- 
2.25.0



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> Do you mean you don’t want in the cc list?
> I think can remove you from cc list if you don’t want in it.

I want you to do exactly as the headers direct you to do.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:05，Nicolas George  写道：
> 
> Nicolas George (12020-04-27):
>> Please run FATE yourself before submitting patches.
> 
> And please heed reply-to headers. It's already annoying enough to have
> you twice in Cc, it's REALLY annoying to get CCed when I explicitly
> asked not to in the proper technical way.
> 
> (Seriously, people, if you want to use crappy e-mail systems, do so, but
> do not annoy third parties with bogus workardounds.)
Do you mean you don’t want in the cc list?
I think can remove you from cc list if you don’t want in it.
> 
> Regards,
> 
> -- 
>  Nicolas George

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午7:04，lance.lmw...@gmail.com 写道：
> 
> On Mon, Apr 27, 2020 at 06:51:47PM +0800, Steven Liu wrote:
>> fix ticket: 8625
>> and add testcase into url for double dot corner case
> 
> I think you need update ./tests/ref/fate/url also.
> 
> make fate-url SAMPLES=../fate-suite
Yes, the fate server have report the result to me.
> 
>> 
>> Suggested-by: Martin Storsjö 
>> Signed-off-by: Steven Liu 
>> ---
>> libavformat/tests/url.c |  3 +++
>> libavformat/url.c   | 21 +++--
>> 2 files changed, 22 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
>> index 5e484fd428..02d0d59aa8 100644
>> --- a/libavformat/tests/url.c
>> +++ b/libavformat/tests/url.c
>> @@ -56,6 +56,7 @@ int main(void)
>> test("/foo/bar", "baz");
>> test("/foo/bar", "../baz");
>> test("/foo/bar", "/baz");
>> +test("/foo/bar", "../../../baz");
>> test("http://server/foo/;, "baz");
>> test("http://server/foo/bar;, "baz");
>> test("http://server/foo/;, "../baz");
>> @@ -65,6 +66,8 @@ int main(void)
>> test("http://server/foo/bar?param=value/with/slashes;, "/baz");
>> test("http://server/foo/bar?param;, "?someparam");
>> test("http://server/foo/bar;, "//other/url");
>> +test("http://server/foo/bar;, "../../../../../other/url");
>> +test("http://server/foo/bar;, "/../../../../../other/url");
>> 
>> printf("\nTesting av_url_split:\n");
>> test2("/foo/bar");
>> diff --git a/libavformat/url.c b/libavformat/url.c
>> index 596fb49cfc..0aa50ab9a7 100644
>> --- a/libavformat/url.c
>> +++ b/libavformat/url.c
>> @@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
>> *base,
>>   const char *rel)
>> {
>> char *sep, *path_query;
>> +char *root, *p;
>> /* Absolute path, relative to the current server */
>> if (base && strstr(base, "://") && rel[0] == '/') {
>> if (base != buf)
>> @@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const 
>> char *base,
>> return;
>> }
>> 
>> +root = p = buf;
>> +/* Get the path root of the url which start by "://" */
>> +if (p && strstr(p, "://")) {
>> +sep = strstr(p, "://");
>> +if (sep) {
>> +sep += 3;
>> +root = strchr(sep, '/');
>> +}
>> +}
>> +
>> /* Remove the file name from the base url */
>> sep = strrchr(buf, '/');
>> +if (sep <= root)
>> +sep = root;
>> +
>> if (sep)
>> sep[1] = '\0';
>> else
>> buf[0] = '\0';
>> while (av_strstart(rel, "../", NULL) && sep) {
>> /* Remove the path delimiter at the end */
>> -sep[0] = '\0';
>> -sep = strrchr(buf, '/');
>> +if (sep > root) {
>> +sep[0] = '\0';
>> +sep = strrchr(buf, '/');
>> +}
>> +
>> /* If the next directory name to pop off is "..", break here */
>> if (!strcmp(sep ? [1] : buf, "..")) {
>> /* Readd the slash we just removed */
>> -- 
>> 2.25.0
>> 
>> 
>> 
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> 
> -- 
> Thanks,
> Limin Wang
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread lance . lmwang

On Mon, Apr 27, 2020 at 06:51:47PM +0800, Steven Liu wrote:
> fix ticket: 8625
> and add testcase into url for double dot corner case

I think you need update ./tests/ref/fate/url also.

 make fate-url SAMPLES=../fate-suite

> 
> Suggested-by: Martin Storsjö 
> Signed-off-by: Steven Liu 
> ---
>  libavformat/tests/url.c |  3 +++
>  libavformat/url.c   | 21 +++--
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
> index 5e484fd428..02d0d59aa8 100644
> --- a/libavformat/tests/url.c
> +++ b/libavformat/tests/url.c
> @@ -56,6 +56,7 @@ int main(void)
>  test("/foo/bar", "baz");
>  test("/foo/bar", "../baz");
>  test("/foo/bar", "/baz");
> +test("/foo/bar", "../../../baz");
>  test("http://server/foo/;, "baz");
>  test("http://server/foo/bar;, "baz");
>  test("http://server/foo/;, "../baz");
> @@ -65,6 +66,8 @@ int main(void)
>  test("http://server/foo/bar?param=value/with/slashes;, "/baz");
>  test("http://server/foo/bar?param;, "?someparam");
>  test("http://server/foo/bar;, "//other/url");
> +test("http://server/foo/bar;, "../../../../../other/url");
> +test("http://server/foo/bar;, "/../../../../../other/url");
>  
>  printf("\nTesting av_url_split:\n");
>  test2("/foo/bar");
> diff --git a/libavformat/url.c b/libavformat/url.c
> index 596fb49cfc..0aa50ab9a7 100644
> --- a/libavformat/url.c
> +++ b/libavformat/url.c
> @@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
> *base,
>const char *rel)
>  {
>  char *sep, *path_query;
> +char *root, *p;
>  /* Absolute path, relative to the current server */
>  if (base && strstr(base, "://") && rel[0] == '/') {
>  if (base != buf)
> @@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const 
> char *base,
>  return;
>  }
>  
> +root = p = buf;
> +/* Get the path root of the url which start by "://" */
> +if (p && strstr(p, "://")) {
> +sep = strstr(p, "://");
> +if (sep) {
> +sep += 3;
> +root = strchr(sep, '/');
> +}
> +}
> +
>  /* Remove the file name from the base url */
>  sep = strrchr(buf, '/');
> +if (sep <= root)
> +sep = root;
> +
>  if (sep)
>  sep[1] = '\0';
>  else
>  buf[0] = '\0';
>  while (av_strstart(rel, "../", NULL) && sep) {
>  /* Remove the path delimiter at the end */
> -sep[0] = '\0';
> -sep = strrchr(buf, '/');
> +if (sep > root) {
> +sep[0] = '\0';
> +sep = strrchr(buf, '/');
> +}
> +
>  /* If the next directory name to pop off is "..", break here */
>  if (!strcmp(sep ? [1] : buf, "..")) {
>  /* Readd the slash we just removed */
> -- 
> 2.25.0
> 
> 
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Nicolas George (12020-04-27):
> Please run FATE yourself before submitting patches.

And please heed reply-to headers. It's already annoying enough to have
you twice in Cc, it's REALLY annoying to get CCed when I explicitly
asked not to in the proper technical way.

(Seriously, people, if you want to use crappy e-mail systems, do so, but
do not annoy third parties with bogus workardounds.)

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> Haha, Patchwork returned it, will fix that.

Please run FATE yourself before submitting patches.

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午6:54，Nicolas George  写道：
> 
> Steven Liu (12020-04-27):
>> and add testcase into url for double dot corner case
> 
> Did you run FATE?
Haha, Patchwork returned it, will fix that.
> 
> Regards,
> 
> -- 
>  Nicolas George

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Nicolas George

Steven Liu (12020-04-27):
> and add testcase into url for double dot corner case

Did you run FATE?

Regards,

-- 
  Nicolas George


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] avformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu

fix ticket: 8625
and add testcase into url for double dot corner case

Suggested-by: Martin Storsjö 
Signed-off-by: Steven Liu 
---
 libavformat/tests/url.c |  3 +++
 libavformat/url.c   | 21 +++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/libavformat/tests/url.c b/libavformat/tests/url.c
index 5e484fd428..02d0d59aa8 100644
--- a/libavformat/tests/url.c
+++ b/libavformat/tests/url.c
@@ -56,6 +56,7 @@ int main(void)
 test("/foo/bar", "baz");
 test("/foo/bar", "../baz");
 test("/foo/bar", "/baz");
+test("/foo/bar", "../../../baz");
 test("http://server/foo/;, "baz");
 test("http://server/foo/bar;, "baz");
 test("http://server/foo/;, "../baz");
@@ -65,6 +66,8 @@ int main(void)
 test("http://server/foo/bar?param=value/with/slashes;, "/baz");
 test("http://server/foo/bar?param;, "?someparam");
 test("http://server/foo/bar;, "//other/url");
+test("http://server/foo/bar;, "../../../../../other/url");
+test("http://server/foo/bar;, "/../../../../../other/url");
 
 printf("\nTesting av_url_split:\n");
 test2("/foo/bar");
diff --git a/libavformat/url.c b/libavformat/url.c
index 596fb49cfc..0aa50ab9a7 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -81,6 +81,7 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
   const char *rel)
 {
 char *sep, *path_query;
+char *root, *p;
 /* Absolute path, relative to the current server */
 if (base && strstr(base, "://") && rel[0] == '/') {
 if (base != buf)
@@ -120,16 +121,32 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
 return;
 }
 
+root = p = buf;
+/* Get the path root of the url which start by "://" */
+if (p && strstr(p, "://")) {
+sep = strstr(p, "://");
+if (sep) {
+sep += 3;
+root = strchr(sep, '/');
+}
+}
+
 /* Remove the file name from the base url */
 sep = strrchr(buf, '/');
+if (sep <= root)
+sep = root;
+
 if (sep)
 sep[1] = '\0';
 else
 buf[0] = '\0';
 while (av_strstart(rel, "../", NULL) && sep) {
 /* Remove the path delimiter at the end */
-sep[0] = '\0';
-sep = strrchr(buf, '/');
+if (sep > root) {
+sep[0] = '\0';
+sep = strrchr(buf, '/');
+}
+
 /* If the next directory name to pop off is "..", break here */
 if (!strcmp(sep ? [1] : buf, "..")) {
 /* Readd the slash we just removed */
-- 
2.25.0



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu



> 2020年4月27日 下午6:30，Martin Storsjö  写道：
> 
> On Mon, 27 Apr 2020, Steven Liu wrote:
> 
>> fix ticket: 8625
>> 
>> Signed-off-by: Steven Liu 
>> ---
>> libavformat/url.c | 22 --
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>> 
>> diff --git a/libavformat/url.c b/libavformat/url.c
>> index 596fb49cfc..e5003f795a 100644
>> --- a/libavformat/url.c
>> +++ b/libavformat/url.c
>> @@ -81,6 +81,8 @@ void ff_make_absolute_url(char *buf, int size, const char 
>> *base,
>>  const char *rel)
>> {
>>char *sep, *path_query;
>> +char *root = NULL;
>> +char *p = NULL;
>>/* Absolute path, relative to the current server */
>>if (base && strstr(base, "://") && rel[0] == '/') {
>>if (base != buf)
> 
> There's a good set of tests for this function in libavformat/tests/url.c - 
> please add a new testcase there for the thing you are implementing (and 
> ideally, also any other closely related corner case).
Ok. Let me try to add them into the test case. Thanks Martin.
> 
> // Martin

Thanks

Steven Liu



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] libavformat/url: check url root node when rel include double dot

2020-04-27 Thread Martin Storsjö


On Mon, 27 Apr 2020, Steven Liu wrote:


fix ticket: 8625

Signed-off-by: Steven Liu 
---
libavformat/url.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/libavformat/url.c b/libavformat/url.c
index 596fb49cfc..e5003f795a 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -81,6 +81,8 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
  const char *rel)
{
char *sep, *path_query;
+char *root = NULL;
+char *p = NULL;
/* Absolute path, relative to the current server */
if (base && strstr(base, "://") && rel[0] == '/') {
if (base != buf)


There's a good set of tests for this function in libavformat/tests/url.c - 
please add a new testcase there for the thing you are implementing (and 
ideally, also any other closely related corner case).


// Martin

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] libavformat/url: check url root node when rel include double dot

2020-04-27 Thread Steven Liu

fix ticket: 8625

Signed-off-by: Steven Liu 
---
 libavformat/url.c | 22 --
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/libavformat/url.c b/libavformat/url.c
index 596fb49cfc..e5003f795a 100644
--- a/libavformat/url.c
+++ b/libavformat/url.c
@@ -81,6 +81,8 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
   const char *rel)
 {
 char *sep, *path_query;
+char *root = NULL;
+char *p = NULL;
 /* Absolute path, relative to the current server */
 if (base && strstr(base, "://") && rel[0] == '/') {
 if (base != buf)
@@ -120,16 +122,32 @@ void ff_make_absolute_url(char *buf, int size, const char 
*base,
 return;
 }
 
+p = buf;
+/* Get the path root of the url which start by "://" */
+if (p && strstr(p, "://")) {
+sep = strstr(p, "://");
+if (sep) {
+sep += 3;
+root = strchr(sep, '/');
+}
+}
+
 /* Remove the file name from the base url */
 sep = strrchr(buf, '/');
+if (sep <= root)
+sep = root;
+
 if (sep)
 sep[1] = '\0';
 else
 buf[0] = '\0';
 while (av_strstart(rel, "../", NULL) && sep) {
 /* Remove the path delimiter at the end */
-sep[0] = '\0';
-sep = strrchr(buf, '/');
+if (sep > root) {
+sep[0] = '\0';
+sep = strrchr(buf, '/');
+}
+
 /* If the next directory name to pop off is "..", break here */
 if (!strcmp(sep ? [1] : buf, "..")) {
 /* Readd the slash we just removed */
-- 
2.25.0



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Complete rewrite of the "fps" video filter section. More accurate.

2020-04-27 Thread Jim DeLaHunt


On 2020-04-27 01:04, Marton Balint wrote:



On Sun, 26 Apr 2020, list+ffmpeg-...@jdlh.com wrote:


From: Jim DeLaHunt 

This is a complete rewrite of the documentation for the "fps" video
filter. It describes the filter's behaviour more clearly and accurately.
I based the rewrite on reading the source code in vf_fps.c closely.


IMHO it is way too verbose now.

Regards,
Marton

I agree, there are a lot more words. On the other hand it now describes 
what the filter actually does, without forcing a user to read the code. 
(And the code isn't straightforward, there is an event-driven style to 
video filter code which takes a bit of learning.)


However, I'm confident it could be improved.

It would be useful for reviewers to comment on:

1. Is there information which should be kept, but expressed with fewer
   words?
2. Is there information which is true description of the filter, but
   which the documentation should not bother to disclose?

I'm of the opinion that FFmpeg users are impeded a lot more by necessary 
facts left out of documentation than by too many words left in.


No code, or other documentation files, are touched by this change.

Signed-off-by: Jim DeLaHunt 

Cheers,

  —Jim DeLaHunt, software engineer, Vancouver, Canada


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v3] libavformat/flacdec: Workaround for truncated metadata picture size

2020-04-27 Thread Mattias Wadman

Hi again, looks ok?

On Fri, Apr 24, 2020 at 12:54 PM Mattias Wadman
 wrote:
>
> lavf flacenc could previously write truncated metadata picture size if
> the picture data did not fit in 24 bits. Detect this by truncting the
> size found inside the picture block and if it matches the block size
> use it and read rest of picture data.
>
> Also only enable this workaround flac files and not ogg files with flac
> METADATA_BLOCK_PICTURE comment.
>
> flacenc was fixed in e447a4d112bcfee10126c54eb4481fa8712957c8
> before the fix a broken flac for reproduction could be generated with:
> ffmpeg -f lavfi -i sine -f lavfi -i color=red:size=2400x2400 -map 0:0 -map 
> 1:0 -c:v:0 bmp -disposition:1 attached_pic -t 1 test.flac
>
> Fixes ticket 6333
> ---
>  libavformat/flac_picture.c   | 35 +++
>  libavformat/flac_picture.h   |  2 +-
>  libavformat/flacdec.c|  2 +-
>  libavformat/oggparsevorbis.c |  2 +-
>  4 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/flac_picture.c b/libavformat/flac_picture.c
> index 81ddf80465..61277e9dee 100644
> --- a/libavformat/flac_picture.c
> +++ b/libavformat/flac_picture.c
> @@ -27,7 +27,7 @@
>  #include "id3v2.h"
>  #include "internal.h"
>
> -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size)
> +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, 
> int truncate_workaround)
>  {
>  const CodecMime *mime = ff_id3v2_mime_tags;
>  enum AVCodecID id = AV_CODEC_ID_NONE;
> @@ -36,7 +36,8 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, 
> int buf_size)
>  GetByteContext g;
>  AVStream *st;
>  int width, height, ret = 0;
> -unsigned int len, type;
> +unsigned int type;
> +uint32_t len, left, trunclen = 0;
>
>  if (buf_size < 34) {
>  av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> short\n");
> @@ -114,16 +115,34 @@ int ff_flac_parse_picture(AVFormatContext *s, uint8_t 
> *buf, int buf_size)
>
>  /* picture data */
>  len = bytestream2_get_be32u();
> -if (len <= 0 || len > bytestream2_get_bytes_left()) {
> -av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> short\n");
> -if (s->error_recognition & AV_EF_EXPLODE)
> -ret = AVERROR_INVALIDDATA;
> -goto fail;
> +
> +left = bytestream2_get_bytes_left();
> +if (len <= 0 || len > left) {
> +// Workaround lavf flacenc bug that allowed writing truncated 
> metadata picture block size if
> +// picture size did not fit in 24 bits
> +if (truncate_workaround && len > left && (len & 0xff) == left) {
> +av_log(s, AV_LOG_INFO, "Correcting truncated metadata picture 
> size from %d to %d\n", left, len);
> +trunclen = len - left;
> +} else {
> +av_log(s, AV_LOG_ERROR, "Attached picture metadata block too 
> short\n");
> +if (s->error_recognition & AV_EF_EXPLODE)
> +ret = AVERROR_INVALIDDATA;
> +goto fail;
> +}
>  }
>  if (!(data = av_buffer_alloc(len + AV_INPUT_BUFFER_PADDING_SIZE))) {
>  RETURN_ERROR(AVERROR(ENOMEM));
>  }
> -bytestream2_get_bufferu(, data->data, len);
> +
> +if (trunclen == 0) {
> +bytestream2_get_bufferu(, data->data, len);
> +} else {
> +// If truncation was detect copy all data from block and read 
> missing bytes
> +// not included in the block size
> +bytestream2_get_bufferu(, data->data, left);
> +if (avio_read(s->pb, data->data + len - trunclen, trunclen) < 
> trunclen)
> +RETURN_ERROR(AVERROR_INVALIDDATA);
> +}
>  memset(data->data + len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>
>  if (AV_RB64(data->data) == PNGSIG)
> diff --git a/libavformat/flac_picture.h b/libavformat/flac_picture.h
> index 4374b6f4f6..61fd0c8806 100644
> --- a/libavformat/flac_picture.h
> +++ b/libavformat/flac_picture.h
> @@ -26,6 +26,6 @@
>
>  #define RETURN_ERROR(code) do { ret = (code); goto fail; } while (0)
>
> -int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size);
> +int ff_flac_parse_picture(AVFormatContext *s, uint8_t *buf, int buf_size, 
> int truncate_workaround);
>
>  #endif /* AVFORMAT_FLAC_PICTURE_H */
> diff --git a/libavformat/flacdec.c b/libavformat/flacdec.c
> index cb516fb1f3..79c05f14bf 100644
> --- a/libavformat/flacdec.c
> +++ b/libavformat/flacdec.c
> @@ -146,7 +146,7 @@ static int flac_read_header(AVFormatContext *s)
>  }
>  av_freep();
>  } else if (metadata_type == FLAC_METADATA_TYPE_PICTURE) {
> -ret = ff_flac_parse_picture(s, buffer, metadata_size);
> +ret = ff_flac_parse_picture(s, buffer, metadata_size, 1);
>  av_freep();
>  if (ret < 0) {
>  av_log(s, AV_LOG_ERROR, "Error parsing attached picture.\n");
> diff --git a/libavformat/oggparsevorbis.c

Re: [FFmpeg-devel] [PATCH] Complete rewrite of the "fps" video filter section. More accurate.

2020-04-27 Thread Marton Balint




On Sun, 26 Apr 2020, list+ffmpeg-...@jdlh.com wrote:


From: Jim DeLaHunt 

This is a complete rewrite of the documentation for the "fps" video
filter. It describes the filter's behaviour more clearly and accurately.
I based the rewrite on reading the source code in vf_fps.c closely.


IMHO it is way too verbose now.

Regards,
Marton



No code, or other documentation files, are touched by this change.

Signed-off-by: Jim DeLaHunt 
---
doc/filters.texi | 167 ++-
1 file changed, 149 insertions(+), 18 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 71a6787289..bd4a1ad2a9 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11139,27 +11139,34 @@ format=pix_fmts=yuv420p|yuv444p|yuv410p
@anchor{fps}
@section fps

-Convert the video to specified constant frame rate by duplicating or dropping
-frames as necessary.
+Generate a video, having the specified constant frame rate, from the frames of 
+the input video, by copying or duplicating or dropping input frames based on 
+their input presentation time stamps (PTSs). The output video has new PTSs. You 
+can choose the method for rounding from input PTS to output PTS.


It accepts the following parameters:
@table @option

@item fps
-The desired output frame rate. The default is @code{25}.
+The output frame rate, as a number of frames per second. This value can be an 
+integer, real, or rational number, or an abbreviation. The default is @code{25}.


@item start_time
-Assume the first PTS should be the given value, in seconds. This allows for
-padding/trimming at the start of stream. By default, no assumption is made
-about the first frame's expected PTS, so no padding or trimming is done.
-For example, this could be set to 0 to pad the beginning with duplicates of
-the first frame if a video stream starts after the audio stream or to trim any
-frames with a negative PTS.
+The time, in seconds from the start of the input stream, which is converted to 
+an input starting PTS value and an output starting PTS value. 
+If set, @var{fps} drops input frames
+which have PTS values less than the input starting PTS. If not set, the input 
+and output starting PTS values are zero, but @var{fps} drops no input frames based 
+on PTS. 
+(See details below.)


@item round
-Timestamp (PTS) rounding method.
+Rounding method to use when calculating output Presentation Timestamp 
+(PTS) integer values from input PTS values. If the calculated output PTS value
+is not exactly an integer, then the method determines which of the two 
+neighbouring integer values to choose.


-Possible values are:
+Possible method names are:
@table @option
@item zero
round towards 0
@@ -11170,43 +11177,167 @@ round towards -infinity
@item up
round towards +infinity
@item near
-round to nearest
+round to nearest (and if exactly at midpoint, away from 0)
@end table
The default is @code{near}.

@item eof_action
-Action performed when reading the last frame.
+Action which @var{fps} takes with the final input frame. The input video passes
+in an ending input PTS, which @var{fps} converts to an ending output PTS. 
+@var{fps} drops any input frames with a PTS at or after this ending PTS.


Possible values are:
@table @option
@item round
-Use same timestamp rounding method as used for other frames.
+Use same rounding method as for other frames, to convert the ending input PTS
+to output PTS.
+
@item pass
-Pass through last frame if input duration has not been reached yet.
+Round the ending input PTS using @code{up}. This can have the effect of passing
+through one last input frame. 
@end table

+
The default is @code{round}.

@end table

-Alternatively, the options can be specified as a flat string:
+Alternatively, the options may be specified as a flat string:
@var{fps}[:@var{start_time}[:@var{round}]].

+@var{fps} generates an output video with integer Presentation Time Stamp (PTS) 
+values which increment by one each output frame, and with a time base set to 
+the inverse of the given frame rate. @var{fps} copies, duplicates, or drops 
+input frames, in sequence, to the output video. It does so according to their 
+input PTS values, as converted to seconds (via the input time base), then 
+rounded to output PTS values. 
+

+@var{fps} sets output PTS values in terms of a time line which starts at
+zero. The integer PTS value multipled by the output time base gives a point 
+in seconds of that output frame on that timeline. If the @var{start_time} 
+parameter is not set, or is zero, the first output frame's PTS value is zero. 
+Otherwise, the first PTS value is the output starting PTS value calculated
+from the @var{start_time} parameter. 
+
+@var{fps} interprets input PTS values in terms of the same time line. It 
+multiplies the input PTS value by the input time base time, to get a frame 
+position in seconds on the time line. It rounds that position to an integer 
+output PTS value. For example, if the input video has a frame rate
+of 30

[FFmpeg-devel] [PATCH] Complete rewrite of the "fps" video filter section. More accurate.

2020-04-27 Thread list+ffmpeg-dev

From: Jim DeLaHunt 

This is a complete rewrite of the documentation for the "fps" video
filter. It describes the filter's behaviour more clearly and accurately.
I based the rewrite on reading the source code in vf_fps.c closely.

No code, or other documentation files, are touched by this change.

Signed-off-by: Jim DeLaHunt 
---
 doc/filters.texi | 167 ++-
 1 file changed, 149 insertions(+), 18 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 71a6787289..bd4a1ad2a9 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -11139,27 +11139,34 @@ format=pix_fmts=yuv420p|yuv444p|yuv410p
 @anchor{fps}
 @section fps
 
-Convert the video to specified constant frame rate by duplicating or dropping
-frames as necessary.
+Generate a video, having the specified constant frame rate, from the frames of 
+the input video, by copying or duplicating or dropping input frames based on 
+their input presentation time stamps (PTSs). The output video has new PTSs. 
You 
+can choose the method for rounding from input PTS to output PTS. 
 
 It accepts the following parameters:
 @table @option
 
 @item fps
-The desired output frame rate. The default is @code{25}.
+The output frame rate, as a number of frames per second. This value can be an 
+integer, real, or rational number, or an abbreviation. The default is 
@code{25}.
 
 @item start_time
-Assume the first PTS should be the given value, in seconds. This allows for
-padding/trimming at the start of stream. By default, no assumption is made
-about the first frame's expected PTS, so no padding or trimming is done.
-For example, this could be set to 0 to pad the beginning with duplicates of
-the first frame if a video stream starts after the audio stream or to trim any
-frames with a negative PTS.
+The time, in seconds from the start of the input stream, which is converted to 
+an input starting PTS value and an output starting PTS value. 
+If set, @var{fps} drops input frames
+which have PTS values less than the input starting PTS. If not set, the input 
+and output starting PTS values are zero, but @var{fps} drops no input frames 
based 
+on PTS. 
+(See details below.)
 
 @item round
-Timestamp (PTS) rounding method.
+Rounding method to use when calculating output Presentation Timestamp 
+(PTS) integer values from input PTS values. If the calculated output PTS value
+is not exactly an integer, then the method determines which of the two 
+neighbouring integer values to choose.
 
-Possible values are:
+Possible method names are:
 @table @option
 @item zero
 round towards 0
@@ -11170,43 +11177,167 @@ round towards -infinity
 @item up
 round towards +infinity
 @item near
-round to nearest
+round to nearest (and if exactly at midpoint, away from 0)
 @end table
 The default is @code{near}.
 
 @item eof_action
-Action performed when reading the last frame.
+Action which @var{fps} takes with the final input frame. The input video passes
+in an ending input PTS, which @var{fps} converts to an ending output PTS. 
+@var{fps} drops any input frames with a PTS at or after this ending PTS.
 
 Possible values are:
 @table @option
 @item round
-Use same timestamp rounding method as used for other frames.
+Use same rounding method as for other frames, to convert the ending input PTS
+to output PTS.
+
 @item pass
-Pass through last frame if input duration has not been reached yet.
+Round the ending input PTS using @code{up}. This can have the effect of passing
+through one last input frame. 
 @end table
+
 The default is @code{round}.
 
 @end table
 
-Alternatively, the options can be specified as a flat string:
+Alternatively, the options may be specified as a flat string:
 @var{fps}[:@var{start_time}[:@var{round}]].
 
+@var{fps} generates an output video with integer Presentation Time Stamp (PTS) 
+values which increment by one each output frame, and with a time base set to 
+the inverse of the given frame rate. @var{fps} copies, duplicates, or drops 
+input frames, in sequence, to the output video. It does so according to their 
+input PTS values, as converted to seconds (via the input time base), then 
+rounded to output PTS values. 
+
+@var{fps} sets output PTS values in terms of a time line which starts at
+zero. The integer PTS value multipled by the output time base gives a point 
+in seconds of that output frame on that timeline. If the @var{start_time} 
+parameter is not set, or is zero, the first output frame's PTS value is zero. 
+Otherwise, the first PTS value is the output starting PTS value calculated
+from the @var{start_time} parameter. 
+
+@var{fps} interprets input PTS values in terms of the same time line. It 
+multiplies the input PTS value by the input time base time, to get a frame 
+position in seconds on the time line. It rounds that position to an integer 
+output PTS value. For example, if the input video has a frame rate
+of 30 fps, a time base of 1/30 seconds, and its first frame has a 
+PTS of 300,

54 matches

Mail list logo