Re: [FFmpeg-devel] [PATCH v2 03/31] lavu/fifo: Add new AVFifo API based upon the notion of element size
Andreas Rheinhardt:
> From: Anton Khirnov
>
> Many AVFifoBuffer users operate on fixed-size elements (e.g. pointers),
> but the current FIFO API deals exclusively in bytes, requiring extra
> complexity in all these callers.
>
> Add a new AVFifo API creating a FIFO with an element size
> that may be larger than a byte. All operations on such a FIFO then
> operate on complete elements.
>
> This API does not reuse AVFifoBuffer and its API at all, but instead uses
> an opaque struct called AVFifo. The AVFifoBuffer API will be deprecated
> in a future commit once all of its users have been switched to the new
> API.
>
> Not reusing AVFifoBuffer also allowed to use the full range of size_t
> from the beginning.
> ---
> doc/APIchanges | 9 ++
> libavutil/fifo.c| 224
> libavutil/fifo.h| 179 +++
> libavutil/version.h | 2 +-
> 4 files changed, 413 insertions(+), 1 deletion(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 8df0364e4c..57a9df9bef 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,15 @@ libavutil: 2021-04-27
>
> API changes, most recent first:
>
> +2022-01-xx - xx - lavu 57.19.100 - fifo.h
> + Add a new FIFO API, which allows setting a FIFO element size.
> + This API operates on these elements rather than on bytes.
> + Add av_fifo_alloc2(), av_fifo_elem_size(), av_fifo_can_read(),
> + av_fifo_can_write(), av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(),
> + av_fifo_write_from_cb(), av_fifo_read(), av_fifo_read_to_cb(),
> + av_fifo_peek(), av_fifo_peek_to_cb(), av_fifo_drain2(), av_fifo_reset2(),
> + av_fifo_freep2().
> +
> 2022-01-04 - 78dc21b123e - lavu 57.16.100 - frame.h
>Add AV_FRAME_DATA_DOVI_METADATA.
>
> diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> index 55621f0dca..0e0d84258f 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -26,6 +26,230 @@
> #include "common.h"
> #include "fifo.h"
>
> +struct AVFifo {
> +uint8_t *buffer;
> +
> +size_t elem_size, nb_elems;
> +size_t offset_r, offset_w;
> +// distinguishes the ambiguous situation offset_r == offset_w
> +intis_empty;
> +};
> +
> +AVFifo *av_fifo_alloc2(size_t nb_elems, size_t elem_size,
> + unsigned int flags)
> +{
> +AVFifo *f;
> +void *buffer;
> +
> +if (!elem_size)
> +return NULL;
> +
> +buffer = av_realloc_array(NULL, nb_elems, elem_size);
After Anton pointed out that it is ill-defined what av_realloc_array()
does in case one requests zero elements (it allocates a single byte,
although it is documented to free the buffer provided to it) this has
been changed to only allocate anything in case a positive number of
elements has been requested:
+void *buffer = NULL;
+
+if (!elem_size)
+return NULL;
+
+if (nb_elems) {
+buffer = av_realloc_array(NULL, nb_elems, elem_size);
+if (!buffer)
+return NULL;
+}
+f = av_mallocz(sizeof(*f));
+if (!f) {
+av_free(buffer);
+return NULL;
+}
I intend to apply this set with this change and his change requested in
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/292349.html
applied (and with another trivial change in qsvdec necessitated by
8ca06a8148) tomorrow unless there are objections.
> +if (!buffer)
> +return NULL;
> +f = av_mallocz(sizeof(*f));
> +if (!f) {
> +av_free(buffer);
> +return NULL;
> +}
> +f->buffer= buffer;
> +f->nb_elems = nb_elems;
> +f->elem_size = elem_size;
> +f->is_empty = 1;
> +
> +return f;
> +}
> +
___
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] cbs_mpeg2_split_fragment(): cache the buffer end
It is constant, so instead of recalculating it every time, only
calculate it once.
Also add a few clarifying comments.
---
libavcodec/cbs_mpeg2.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index dfea12ef1b..d3204b885a 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -144,12 +144,12 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
CodedBitstreamFragment *frag,
int header)
{
-const uint8_t *start;
+const uint8_t *start = frag->data;
+const uint8_t * const buf_end = frag->data + frag->data_size;
uint32_t start_code = -1;
int err;
-start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
- &start_code);
+start = avpriv_find_start_code(start, buf_end, &start_code);
if (start_code >> 8 != 0x01) {
// No start code found.
return AVERROR_INVALIDDATA;
@@ -165,10 +165,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
// start code in any way (as e.g. happens when there is a
// Sequence End unit at the very end of a packet).
start_code = UINT32_MAX;
-end = avpriv_find_start_code(start--, frag->data + frag->data_size,
- &start_code);
-
-// start points to the byte containing the start_code_identifier
+end = avpriv_find_start_code(start, buf_end, &start_code);
+start--;
+// decrement so start points to the byte containing the
start_code_identifier
// (may be the last byte of fragment->data); end points to the byte
// following the byte containing the start code identifier (or to
// the end of fragment->data).
@@ -178,6 +177,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
unit_size = (end - 4) - start;
} else {
// We didn't find a start code, so this is the final unit.
+ // There is no start code to remove from end, hence not (end - 4).
unit_size = end - start;
}
--
2.32.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 02/13] avpriv_find_start_code(): readability enhancement part 1
Scott Theisen: > No functional change. > --- > libavcodec/utils.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index b19befef21..cb4437edc2 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -967,10 +967,14 @@ const uint8_t *avpriv_find_start_code(const uint8_t > *av_restrict p, > } > } > > -p = FFMIN(p, end) - 4; > -*state = AV_RB32(p); > +if (p > end) > +p = end; > +// this will cause the last 4 bytes before end to be read, > +// i.e. no out of bounds memory access occurs > > -return p + 4; > +*state = AV_RB32(p - 4); // read the previous 4 bytes > + > +return p; > } > > AVCPBProperties *av_cpb_properties_alloc(size_t *size) Where exactly is the readability enhancement supposed to be? I only see the opposite: The earlier code spoke for itself; not this simplicity is obscured by lots of comments. Having to parse lots of comments makes the code harder to read. This is also my impression with your other clarification patches. - 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] avutil: use getauxval on linux/android for CPU capabilities
On Fri, Feb 4, 2022 at 2:20 PM Martin Storsjö wrote: > On Fri, 4 Feb 2022, Aman Karmani wrote: > > > From: Aman Karmani > > > > fixes #6578 > > > > Signed-off-by: Aman Karmani > > --- > > libavutil/arm/cpu.c | 17 ++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > It would be good if the commit message actually explained the upsides to > doing this. > > It's also important to point out that this function didn't use to exist in > all Android versions (hence the /proc/cpuinfo parsing). The point when the > function appeared (Android 4.4, API 20, according to the referenced trac > issue) is kinda far in the past today, but I'm not sure if strictly all > users of the library have stopped supporting older versions still - at > least for some use cases I've heard of recently, Android 4.1 was still > supported. > > If coupled with a configure check for the function, so that users > explicitly targeting an older version that lack it wouldn't get it, I > guess this could be more easily acceptable. Thanks for the review. I discovered some older Linux arm toolchains (< glibc 2.16) are also missing the function, so a configure check seems prudent. Sent v2 patch with the requested changes. > > // 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 v2] avutil: use getauxval for CPU capabilities on linux/android ARM
From: Aman Karmani
getauxval is faster, and works when procfs is not mounted
note that support on Android was added in 4.4 (API 20)
fixes #6578
Signed-off-by: Aman Karmani
---
configure | 1 +
libavutil/arm/cpu.c | 24 +---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 5b19a35f59..d21cfb4cbd 100755
--- a/configure
+++ b/configure
@@ -6265,6 +6265,7 @@ check_func_headers lzo/lzo1x.h lzo1x_999_compress
check_func_headers mach/mach_time.h mach_absolute_time
check_func_headers stdlib.h getenv
check_func_headers sys/stat.h lstat
+check_func_headers sys/auxv.h getauxval
check_func_headers windows.h GetModuleHandle
check_func_headers windows.h GetProcessAffinityMask
diff --git a/libavutil/arm/cpu.c b/libavutil/arm/cpu.c
index 81e85e2525..c84a655c37 100644
--- a/libavutil/arm/cpu.c
+++ b/libavutil/arm/cpu.c
@@ -38,6 +38,10 @@
#include
#include "libavutil/avstring.h"
+#if HAVE_GETAUXVAL
+#include
+#endif
+
#define AT_HWCAP16
/* Relevant HWCAP values from kernel headers */
@@ -48,6 +52,19 @@
#define HWCAP_VFPv3 (1 << 13)
#define HWCAP_TLS (1 << 15)
+static int get_auxval(uint32_t *hwcap)
+{
+#if HAVE_GETAUXVAL
+unsigned long ret = getauxval(AT_HWCAP);
+if (ret == 0)
+return -1;
+*hwcap = ret;
+return 0;
+#else
+return -1;
+#endif
+}
+
static int get_hwcap(uint32_t *hwcap)
{
struct { uint32_t a_type; uint32_t a_val; } auxv;
@@ -106,9 +123,10 @@ int ff_get_cpu_flags_arm(void)
int flags = CORE_CPU_FLAGS;
uint32_t hwcap;
-if (get_hwcap(&hwcap) < 0)
-if (get_cpuinfo(&hwcap) < 0)
-return flags;
+if (get_auxval(&hwcap) < 0)
+if (get_hwcap(&hwcap) < 0)
+if (get_cpuinfo(&hwcap) < 0)
+return flags;
#define check_cap(cap, flag) do { \
if (hwcap & HWCAP_ ## cap) \
--
2.33.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 08/13] avpriv_find_start_code(): add parameter for ignoring history
Scott Theisen:
> If true, this skips the for loop at the beginning of avpriv_find_start_code().
>
> If the state/start_code input is a local variable and only one buffer is used,
> then no history is needed.
>
> In loops and inline functions: if ignoring history, don't initialize
> start_code,
> so it isn't reset twice each time.
>
> cbs_mpeg2.c needs further changes to use output_only = true.
>
> Use output_only = 0 for no functional change. For example, if the
> state/start_code
> is passed into the function calling avpriv_find_start_code().
> ---
> libavcodec/cavsdec.c | 2 +-
> libavcodec/cbs_mpeg2.c| 4 ++--
> libavcodec/extract_extradata_bsf.c| 4 ++--
> libavcodec/h264_parser.c | 2 +-
> libavcodec/internal.h | 9 -
> libavcodec/mpeg12.c | 2 +-
> libavcodec/mpeg12dec.c| 9 -
> libavcodec/mpeg4_unpack_bframes_bsf.c | 3 +--
> libavcodec/mpegvideo_parser.c | 5 ++---
> libavcodec/remove_extradata_bsf.c | 8
> libavcodec/utils.c| 14 +-
> libavcodec/vc1_common.h | 4 ++--
> libavformat/avidec.c | 6 +++---
> libavformat/avs2dec.c | 2 +-
> libavformat/avs3dec.c | 2 +-
> libavformat/cavsvideodec.c| 2 +-
> libavformat/mpegtsenc.c | 4 ++--
> libavformat/mpegvideodec.c| 2 +-
> libavformat/mxfenc.c | 2 +-
> libavformat/rtpenc_mpv.c | 4 ++--
> 20 files changed, 53 insertions(+), 37 deletions(-)
>
> diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
> index a62177d520..c4c78cd534 100644
> --- a/libavcodec/cavsdec.c
> +++ b/libavcodec/cavsdec.c
> @@ -1248,7 +1248,7 @@ static int cavs_decode_frame(AVCodecContext *avctx,
> void *data, int *got_frame,
> buf_ptr = buf;
> buf_end = buf + buf_size;
> for(;;) {
> -buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
> +buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc, 1);
> if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
> if (!h->stc)
> av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index eb45929132..d41477620e 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -151,7 +151,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> int err, i, final = 0;
>
> start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> - &start_code);
> + &start_code, 1);
> if (!avpriv_start_code_is_valid(start_code)) {
> // No start code found.
> return AVERROR_INVALIDDATA;
> @@ -169,7 +169,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> }
>
> end = avpriv_find_start_code(start--, frag->data + frag->data_size,
> - &start_code);
> + &start_code, 0);
>
> // start points to the byte containing the start_code_identifier
> // (may be the last byte of fragment->data); end points to the byte
> diff --git a/libavcodec/extract_extradata_bsf.c
> b/libavcodec/extract_extradata_bsf.c
> index 4df1c97139..1a3ddceff2 100644
> --- a/libavcodec/extract_extradata_bsf.c
> +++ b/libavcodec/extract_extradata_bsf.c
> @@ -237,7 +237,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx,
> AVPacket *pkt,
> int has_extradata = 0, extradata_size = 0;
>
> while (ptr < end) {
> -ptr = avpriv_find_start_code(ptr, end, &state);
> +ptr = avpriv_find_start_code(ptr, end, &state, 1);
> if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
> has_extradata = 1;
> } else if (has_extradata && avpriv_start_code_is_valid(state)) {
> @@ -300,7 +300,7 @@ static int extract_extradata_mpeg4(AVBSFContext *ctx,
> AVPacket *pkt,
> uint32_t state = UINT32_MAX;
>
> while (ptr < end) {
> -ptr = avpriv_find_start_code(ptr, end, &state);
> +ptr = avpriv_find_start_code(ptr, end, &state, 1);
> if (state == 0x1B3 || state == 0x1B6) {
> if (ptr - pkt->data > 4) {
> *size = ptr - 4 - pkt->data;
> diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
> index 4002bcad77..0ca411c592 100644
> --- a/libavcodec/h264_parser.c
> +++ b/libavcodec/h264_parser.c
> @@ -72,7 +72,7 @@ static int find_start_code(const uint8_t *buf, int buf_size,
> {
> uint32_t state = -1;
>
> -buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1,
> &state) - buf - 1;
> +buf_index = avpriv_find_start_code(buf + buf_index, buf + next_avc + 1,
> &state, 1) - buf - 1;
>
> return FFMIN(buf_ind
Re: [FFmpeg-devel] [PATCH v2 07/13] avpriv_find_start_code(): constify pointer parameters
On 2/5/22 00:49, Andreas Rheinhardt wrote: - * @param[in,out] start_code A reference to a mutable @c uint32_t. + * @param[in,out] start_code A constant pointer (reference) to a mutable @c uint32_t. There are no references in C. A pointer is a type of reference. However, I agree "A pointer to a mutable @c uint32_t." would be clearer. Documenting restrictions on the callee that are irrelevant to the caller in a public header seems weird. Fair enough, the added consts shall be limited to libavcodec/utils.c. -Scott ___ 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 01/13] avcodec/internal.h: create avpriv_start_code_is_valid()
On 2/5/22 00:42, Andreas Rheinhardt wrote: a) We use the avpriv prefix for things that should be exported from one library to be used in other libraries, but not for public use. Therefore the avpriv prefix is inappropriate here as this function is static. And it makes a very long name. So since this is static av_always_inline, it should just be start_code_is_valid()? b) internal.h is the wrong header for this: There are more start codes than 00 00 01. That's why I sent the patch to move avpriv_find_start_code() to libavcodec/startcode.h. I only put it there because that is where avpriv_find_start_code() is declared, so I knew it was already included. Should I move the definition and declaration of avpriv_find_start_code() to startcode.(c|h)? c) I am not sure that the new code is equivalent to the old one in all instances: mpeg12dec.c checks for "start_code > 0x1ff" to mean "no valid start code", yet if the buffer ended with anything in the range 0x00-0xff it would be considered a start code before this patch and now it would no longer be a start code. I don't think this is a bad change, though, but it should be noted in the commit message. I'll have to look at them all again and I'll add that to the commit message. -Scott ___ 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 07/13] avpriv_find_start_code(): constify pointer parameters
Scott Theisen:
> Have the compiler enforce not changing the addresses these parameters point
> to.
>
> No functional change.
> ---
> libavcodec/internal.h | 6 +++---
> libavcodec/utils.c| 4 ++--
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 94c41aef0b..dadd8d4a10 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -313,7 +313,7 @@ static av_always_inline int
> avpriv_start_code_is_valid(uint32_t start_code) {
> * @param[in] end A pointer to the past-the-end memory address for the
> buffer
> * given by @p p. @p p must be ≤ @p end.
> *
> - * @param[in,out] start_code A reference to a mutable @c uint32_t.
> + * @param[in,out] start_code A constant pointer (reference) to a mutable @c
> uint32_t.
There are no references in C.
> * As input: For no history preset to @c ~0 , otherwise
> preset to the last
> *returned start code to enable detecting start codes
> across
> *buffer boundaries.
> @@ -325,8 +325,8 @@ static av_always_inline int
> avpriv_start_code_is_valid(uint32_t start_code) {
> * if no start code was found.
> */
> const uint8_t *avpriv_find_start_code(const uint8_t *p,
> - const uint8_t *end,
> - uint32_t *start_code);
> + const uint8_t * const end,
> + uint32_t * const start_code);
>
> int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 80ccde023f..cf92d29f67 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -941,8 +941,8 @@ void ff_thread_report_progress2(AVCodecContext *avctx,
> int field, int thread, in
> #endif
>
> const uint8_t *avpriv_find_start_code(const uint8_t *av_restrict p,
> - const uint8_t *end,
> - uint32_t *av_restrict start_code)
> + const uint8_t * const end,
> + uint32_t * const av_restrict
> start_code)
> {
> av_assert0(p <= end);
> if (p >= end)
Documenting restrictions on the callee that are irrelevant to the caller
in a public header seems weird.
- 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 01/13] avcodec/internal.h: create avpriv_start_code_is_valid()
Scott Theisen:
> ---
> libavcodec/cavsdec.c | 2 +-
> libavcodec/cbs_mpeg2.c | 4 ++--
> libavcodec/extract_extradata_bsf.c | 2 +-
> libavcodec/internal.h | 15 +++
> libavcodec/mpeg12.c| 2 +-
> libavcodec/mpeg12dec.c | 2 +-
> libavcodec/mpegvideo_parser.c | 2 +-
> libavcodec/remove_extradata_bsf.c | 8 +++-
> libavcodec/vaapi_vc1.c | 2 +-
> libavcodec/vc1_common.h| 4 +---
> libavcodec/vc1dec.c| 2 +-
> libavformat/avs2dec.c | 4 ++--
> libavformat/avs3dec.c | 4 ++--
> libavformat/cavsvideodec.c | 2 +-
> libavformat/mpegvideodec.c | 2 +-
> libavformat/rtpenc_mpv.c | 2 +-
> 16 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
> index 692c77eb39..a62177d520 100644
> --- a/libavcodec/cavsdec.c
> +++ b/libavcodec/cavsdec.c
> @@ -1249,7 +1249,7 @@ static int cavs_decode_frame(AVCodecContext *avctx,
> void *data, int *got_frame,
> buf_end = buf + buf_size;
> for(;;) {
> buf_ptr = avpriv_find_start_code(buf_ptr, buf_end, &stc);
> -if ((stc & 0xFE00) || buf_ptr == buf_end) {
> +if (!avpriv_start_code_is_valid(stc) || buf_ptr == buf_end) {
> if (!h->stc)
> av_log(h->avctx, AV_LOG_WARNING, "no frame decoded\n");
> return FFMAX(0, buf_ptr - buf);
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 26400f279f..eb45929132 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -152,7 +152,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
>
> start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> &start_code);
> -if (start_code >> 8 != 0x01) {
> +if (!avpriv_start_code_is_valid(start_code)) {
> // No start code found.
> return AVERROR_INVALIDDATA;
> }
> @@ -175,7 +175,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> // (may be the last byte of fragment->data); end points to the byte
> // following the byte containing the start code identifier (or to
> // the end of fragment->data).
> -if (start_code >> 8 == 0x01) {
> +if (avpriv_start_code_is_valid(start_code)) {
> // Unit runs from start to the beginning of the start code
> // pointed to by end (including any padding zeroes).
> unit_size = (end - 4) - start;
> diff --git a/libavcodec/extract_extradata_bsf.c
> b/libavcodec/extract_extradata_bsf.c
> index dbcb8508b0..4df1c97139 100644
> --- a/libavcodec/extract_extradata_bsf.c
> +++ b/libavcodec/extract_extradata_bsf.c
> @@ -240,7 +240,7 @@ static int extract_extradata_vc1(AVBSFContext *ctx,
> AVPacket *pkt,
> ptr = avpriv_find_start_code(ptr, end, &state);
> if (state == VC1_CODE_SEQHDR || state == VC1_CODE_ENTRYPOINT) {
> has_extradata = 1;
> -} else if (has_extradata && IS_MARKER(state)) {
> +} else if (has_extradata && avpriv_start_code_is_valid(state)) {
> extradata_size = ptr - 4 - pkt->data;
> break;
> }
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 72ca1553f6..005f308b70 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -285,6 +285,21 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
>
> int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
>
> +/**
> + * @brief Test whether a start code found by avpriv_find_start_code() is
> valid.
> + *
> + * Use this to test the validity of a start code especially if a start code
> can
> + * be at the end of the buffer, where testing the return value of
> avpriv_find_start_code()
> + * would incorrectly imply that the start code is invalid (since the
> returned value
> + * equals @c end ).
> + *
> + * @param[in] start_code The start code to test.
> + * @return A boolean that is true if and only if @p start_code is
> valid
> + */
> +static av_always_inline int avpriv_start_code_is_valid(uint32_t start_code) {
> +return (start_code & 0xFF00) == 0x100;
> +}
> +
> const uint8_t *avpriv_find_start_code(const uint8_t *p,
>const uint8_t *end,
>uint32_t *state);
> diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
> index 58e03c05d4..e45bc74479 100644
> --- a/libavcodec/mpeg12.c
> +++ b/libavcodec/mpeg12.c
> @@ -217,7 +217,7 @@ int ff_mpeg1_find_frame_end(ParseContext *pc, const
> uint8_t *buf, int buf_size,
> pc->frame_start_found = 0;
> if (pc->frame_start_found < 4 && state == EXT_START_CODE)
> pc->frame_start_found++;
> -if (pc->frame_start_found == 4 && (state & 0xF
[FFmpeg-devel] [PATCH v2 3/3] avformat/udp: remove IPPROTO_IPV6 macro
From: Limin Wang Signed-off-by: Limin Wang --- libavformat/udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/udp.c b/libavformat/udp.c index 34488d6..0b078d4 100644 --- a/libavformat/udp.c +++ b/libavformat/udp.c @@ -175,7 +175,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL, cmd = IP_MULTICAST_TTL; break; #endif -#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS) +#ifdef IPV6_MULTICAST_HOPS case AF_INET6: protocol = IPPROTO_IPV6; cmd = IPV6_MULTICAST_HOPS; -- 1.8.3.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".
[FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility
From: Limin Wang
Suggested by zhilizhao, vlc project has solved the compatibility by
the same way, so I borrowed the comments from vlc project.
Fix #ticket9449
Signed-off-by: Limin Wang
---
libavformat/udp.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/libavformat/udp.c b/libavformat/udp.c
index 3dc79eb..34488d6 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
{
int protocol, cmd;
+/* There is some confusion in the world whether IP_MULTICAST_TTL
+ * takes a byte or an int as an argument.
+ * BSD seems to indicate byte so we are going with that and use
+ * int as a fallback to be safe */
switch (addr->sa_family) {
#ifdef IP_MULTICAST_TTL
case AF_INET:
@@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
}
if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
-return ff_neterrno();
+/* BSD compatibility */
+unsigned char ttl;
+
+ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
+ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
+if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
+return ff_neterrno();
+}
}
return 0;
--
1.8.3.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".
[FFmpeg-devel] [PATCH v2 1/3] avformat/udp: use one setsockopt for ipv4/ipv6
From: Limin Wang
Signed-off-by: Limin Wang
---
libavformat/udp.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..3dc79eb 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
struct sockaddr *addr,
void *logctx)
{
+int protocol, cmd;
+
+switch (addr->sa_family) {
#ifdef IP_MULTICAST_TTL
-if (addr->sa_family == AF_INET) {
-if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL,
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR,
"setsockopt(IP_MULTICAST_TTL)");
-return ff_neterrno();
-}
-}
+case AF_INET:
+protocol = IPPROTO_IP;
+cmd = IP_MULTICAST_TTL;
+break;
#endif
#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
-if (addr->sa_family == AF_INET6) {
-if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL,
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR,
"setsockopt(IPV6_MULTICAST_HOPS)");
+case AF_INET6:
+protocol = IPPROTO_IPV6;
+cmd = IPV6_MULTICAST_HOPS;
+break;
+#endif
+default:
+errno = EAFNOSUPPORT;
return ff_neterrno();
-}
}
-#endif
+
+if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
+return ff_neterrno();
+}
+
return 0;
}
--
1.8.3.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] [PATCH v2 13/13] cbs_mpeg2.c: improve readability of start code search (part 3)
On 2/4/22 22:54, Andreas Rheinhardt wrote:
I disagree that this is the true loop condition that just needs to be
revealed; in fact, this is basically the loop condition from before
fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to
deal with size one units at the end. I considered this and chose the
current one because it leads to less code duplication for this special case.
Anyway, now that I have taken another look at this and I finally found
the true loop condition: Is there a unit that we have not added to the
fragment yet? This is easily implementable if one always resets the
start code, so that there being an outstanding unit is equivalent to the
start code being valid.
Looking at your patch series again, I agree your changes to cbs_mpeg2.c
are clearer.
However, I think my changes from patch 11 are a further helpful
clarification (ignoring the index and loop changes (since you already
did that) and the "redundant" comment):
@@ -144,23 +144,24 @@ static int
cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
CodedBitstreamFragment *frag,
int header)
{
- const uint8_t *start, *end;
+ const uint8_t *start = frag->data, *end;
+ const uint8_t * const buf_end = frag->data + frag->data_size;
CodedBitstreamUnitType unit_type;
uint32_t start_code = -1;
size_t unit_size;
- int err, i = 0, final = 0;
+ int err, final = 0;
+ int i = -1; // offset for pre-increment
- start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
- &start_code, 1);
+ start = avpriv_find_start_code(start, buf_end, &start_code, 1);
if (!avpriv_start_code_is_valid(start_code)) {
// No start code found.
return AVERROR_INVALIDDATA;
}
- while (!final) {
+ do {
unit_type = start_code & 0xff;
- if (start == frag->data + frag->data_size) {
+ if (start == buf_end) {
// The last four bytes form a start code which constitutes
// a unit of its own. In this situation avpriv_find_start_code
// won't modify start_code at all so modify start_code so that
@@ -168,10 +169,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
start_code = 0;
}
- end = avpriv_find_start_code(start--, frag->data + frag->data_size,
- &start_code, 0);
-
- // start points to the byte containing the start_code_identifier
+ end = avpriv_find_start_code(start, buf_end, &start_code, 0);
+ start--;
+ // decrement so start points to the byte containing the
start_code_identifier
// (may be the last byte of fragment->data); end points to the byte
// following the byte containing the start code identifier (or to
// the end of fragment->data).
Should I submit a v3 patch series which only includes patches 1-9? (That
is only the avpriv_find_start_code() changes, since 10-13 were
cbs_mpeg2.c and separate but related.)
Regards,
Scott
___
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 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop
On 2/4/22 23:06, Andreas Rheinhardt wrote: Scott Theisen: On 2/4/22 20:48, Andreas Rheinhardt wrote: I disagree that this enhances clarity: When using a counter, a for loop is the most natural. The counter is not used as the loop condition, so it is not natural. Also, you removed the counter and made this a do while loop in your own patch series. Yes, after the counter has been removed, using a for-loop stopped being natural. It is not really a for-loop, though, since it has no loop condition and is thus an infinite loop that just happens to be abusing the for-loop syntax. -Scott ___ 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 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop
Scott Theisen: > On 2/4/22 20:48, Andreas Rheinhardt wrote: >> I disagree that this enhances clarity: When using a counter, a for loop >> is the most natural. > > The counter is not used as the loop condition, so it is not natural. > Also, you removed the counter and made this a do while loop in your own > patch series. Yes, after the counter has been removed, using a for-loop stopped being natural. - 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 12/13] cbs_mpeg2.c: improve readability of start code search (part 2)
On 2/4/22 21:24, Andreas Rheinhardt wrote: end is now uninitialized in case of a unit consisting solely of a Sequence End. I hadn't noticed that. However, I immediately restored it in the next patch. ___ 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 13/13] cbs_mpeg2.c: improve readability of start code search (part 3)
Scott Theisen:
> Separate from part 2 for a clearer diff.
>
> Now the true loop condition has been revealed: start < buf_end
>
> Clarify loop by moving the detection of sequence_end_codes out of the loop.
> See also commit fd93d5efe64206d5f1bce8c702602353444c0c1a regarding
> sequence_end_codes
> ---
> libavcodec/cbs_mpeg2.c | 31 ++-
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index bf95fb7546..53aa0ed3f6 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -149,7 +149,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> CodedBitstreamUnitType unit_type;
> uint32_t start_code = -1;
> size_t unit_size;
> -int err, final = 0;
> +int err;
> int i = -1; // offset for pre-increment
>
> start = avpriv_find_start_code(start, buf_end, &start_code, 1);
> @@ -161,16 +161,7 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> do {
> unit_type = start_code & 0xff;
>
> -if (start == buf_end) {
> -// The last four bytes form a start code which constitutes
> -// a unit of its own. In this situation avpriv_find_start_code
> -// won't modify start_code at all so modify start_code so that
> -// the next unit will be treated as the last unit.
> -start_code = 0;
> -}
> -else {
> -end = avpriv_find_start_code(start, buf_end, &start_code, 1);
> -}
> +end = avpriv_find_start_code(start, buf_end, &start_code, 1);
> start--;
> // decrement so start points to the byte containing the
> start_code_identifier
> // (may be the last byte of fragment->data); end points to the byte
> @@ -182,8 +173,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> unit_size = (end - 4) - start;
> } else {
> // We didn't find a start code, so this is the final unit.
> + // There is no start code to remove from end, hence not (end - 4).
> unit_size = end - start;
> - final = 1;
> }
>
> err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
> @@ -193,7 +184,21 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> return err;
>
> start = end;
> -} while (!final);
> +} while (start < buf_end);
> +
> +if (avpriv_start_code_is_valid(start_code)) {
> +// The last four bytes form a start code which constitutes
> +// a unit of its own, with size 1.
> +
> +start--; // since start == buf_end because of the loop condition,
> +// decrement so start points to the byte containing the
> start_code_identifier
> +err = ff_cbs_insert_unit_data(frag, ++i, start_code & 0xFF,
> + (uint8_t*)start /* cast away the const
> to match parameter type */,
> + 1, frag->data_ref);
> +if (err < 0) {
> +return err;
> +}
> +}
>
> return 0;
> }
I disagree that this is the true loop condition that just needs to be
revealed; in fact, this is basically the loop condition from before
fd93d5efe64206d5f1bce8c702602353444c0c1a, just with an added block to
deal with size one units at the end. I considered this and chose the
current one because it leads to less code duplication for this special case.
Anyway, now that I have taken another look at this and I finally found
the true loop condition: Is there a unit that we have not added to the
fragment yet? This is easily implementable if one always resets the
start code, so that there being an outstanding unit is equivalent to the
start code being valid.
- 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 11/13] cbs_mpeg2.c: improve readability of start code search (part 1)
On 2/4/22 21:16, Andreas Rheinhardt wrote: Using a pre-increment is unnatural (i is supposed to be the number of units of the fragment and so it should naturally be incremented after a unit has been successfully added to the fragment) and impairs clarity. It *is* incremented after a unit has been successfully added, on the *next* iteration. redundant comment Debatable, the variable is already a uint8_t*, albeit const qualified. The purpose of comments is to explain. ___ 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 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop
On 2/4/22 20:48, Andreas Rheinhardt wrote: I disagree that this enhances clarity: When using a counter, a for loop is the most natural. The counter is not used as the loop condition, so it is not natural. Also, you removed the counter and made this a do while loop in your own patch series. ___ 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/rtpdec_rfc4175: fix interlace format
On Wed, Feb 02, 2022 at 03:45:10PM -0500, Patrick Keroulas wrote:
> In previous state, a new frame was allocated on each timestamp step,
> i.e. each frame/field transition. However, for interlace, a new frame
> should be allocated on 1st field, completed with the 2nd and finally
> freed.
>
> This commit fixes the frame allocation and the detection of missing RTP
> markers.
>
> Signed-off-by: Patrick Keroulas
> ---
> libavformat/rtpdec_rfc4175.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/rtpdec_rfc4175.c b/libavformat/rtpdec_rfc4175.c
> index 8e73c07838..83abe499f8 100644
> --- a/libavformat/rtpdec_rfc4175.c
> +++ b/libavformat/rtpdec_rfc4175.c
> @@ -234,20 +234,21 @@ static int rfc4175_handle_packet(AVFormatContext *ctx,
> PayloadContext *data,
> uint8_t *dest;
>
> if (*timestamp != data->timestamp) {
> -if (data->frame) {
> +if (data->frame && (!data->interlaced || data->field)) {
> /*
> - * if we're here, it means that two RTP packets didn't have the
> - * same timestamp, which is a sign that they were packets from
> two
> - * different frames, but we didn't get the flag RTP_FLAG_MARKER
> on
> - * the first one of these frames (last packet of a frame).
> - * Finalize the previous frame anyway by filling the AVPacket.
> + * if we're here, it means that we missed the cue to return
> + * the previous AVPacket, that cue being the RTP_FLAG_MARKER
> + * in the last packet of either the previous frame (progressive)
> + * or the previous second field (interlace). Let's finalize the
> + * previous frame (or pair of fields) anyway by filling the
> AVPacket.
> */
> av_log(ctx, AV_LOG_ERROR, "Missed previous RTP Marker\n");
> missed_last_packet = 1;
> rfc4175_finalize_packet(data, pkt, st->index);
> }
>
> -data->frame = av_malloc(data->frame_size);
> +if (!data->frame)
> +data->frame = av_malloc(data->frame_size);
>
> data->timestamp = *timestamp;
>
> --
> 2.25.1
>
applied, thanks.
--
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] Refactoring UUID functionality
On 21/1/22 03:45, Pierre-Anthony Lemieux wrote:
Hi all,
It was recently suggested that UUID functionality in the codebase
could be refactored into a single library.
Below is short explainer.
I would appreciate your review/feedback before I/we start writing code.
Best,
-- Pierre
I have no complaints, the approach looks good.
From what Pierre, Lynne, and myself discussed privately, basically the plan is
to:
* Rip out uuid_{un,}parse() from libuuid into av_uuid_{un,}parse(), and
* Use it to dedup the existing ad-hoc UUID handling littered around the place.
___
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 12/13] cbs_mpeg2.c: improve readability of start code search (part 2)
Scott Theisen:
> If the last four bytes form a valid start code, the loop would run another
> time.
> Since (start == buf_end), start_code is invalidated so the loop terminates.
>
> However, calling avpriv_find_start_code() with p == end, it will immediately
> return due to the zero size buffer. Thus the history is not needed.
> ---
> libavcodec/cbs_mpeg2.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index f2efedde5d..bf95fb7546 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -168,8 +168,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> // the next unit will be treated as the last unit.
> start_code = 0;
> }
> -
> -end = avpriv_find_start_code(start, buf_end, &start_code, 0);
> +else {
> +end = avpriv_find_start_code(start, buf_end, &start_code, 1);
> +}
> start--;
> // decrement so start points to the byte containing the
> start_code_identifier
> // (may be the last byte of fragment->data); end points to the byte
end is now uninitialized in case of a unit consisting solely of a
Sequence End.
- 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 11/13] cbs_mpeg2.c: improve readability of start code search (part 1)
Scott Theisen:
> ff_cbs_insert_unit_data() does not modify the data or data_size fields of
> the CodedBitstreamFragment. It also does not modify the value pointed to
> by start. (We don't need that byte in this function anymore, anyway.)
> ---
> libavcodec/cbs_mpeg2.c | 26 +-
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index afe78eef9a..f2efedde5d 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -144,23 +144,24 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> CodedBitstreamFragment *frag,
> int header)
> {
> -const uint8_t *start, *end;
> +const uint8_t *start = frag->data, *end;
> +const uint8_t * const buf_end = frag->data + frag->data_size;
> CodedBitstreamUnitType unit_type;
> uint32_t start_code = -1;
> size_t unit_size;
> -int err, i = 0, final = 0;
> +int err, final = 0;
> +int i = -1; // offset for pre-increment
Using a pre-increment is unnatural (i is supposed to be the number of
units of the fragment and so it should naturally be incremented after a
unit has been successfully added to the fragment) and impairs clarity.
>
> -start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> - &start_code, 1);
> +start = avpriv_find_start_code(start, buf_end, &start_code, 1);
> if (!avpriv_start_code_is_valid(start_code)) {
> // No start code found.
> return AVERROR_INVALIDDATA;
> }
>
> -while (!final) {
> +do {
> unit_type = start_code & 0xff;
>
> -if (start == frag->data + frag->data_size) {
> +if (start == buf_end) {
> // The last four bytes form a start code which constitutes
> // a unit of its own. In this situation avpriv_find_start_code
> // won't modify start_code at all so modify start_code so that
> @@ -168,10 +169,9 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> start_code = 0;
> }
>
> -end = avpriv_find_start_code(start--, frag->data + frag->data_size,
> - &start_code, 0);
> -
> -// start points to the byte containing the start_code_identifier
> +end = avpriv_find_start_code(start, buf_end, &start_code, 0);
> +start--;
> +// decrement so start points to the byte containing the
> start_code_identifier
> // (may be the last byte of fragment->data); end points to the byte
> // following the byte containing the start code identifier (or to
> // the end of fragment->data).
> @@ -185,14 +185,14 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> final = 1;
> }
>
> -err = ff_cbs_insert_unit_data(frag, i, unit_type, (uint8_t*)start,
> +err = ff_cbs_insert_unit_data(frag, ++i, unit_type,
> + (uint8_t*)start /* cast away the const
> to match parameter type */,
redundant comment
>unit_size, frag->data_ref);
> if (err < 0)
> return err;
>
> start = end;
> -i++;
> -}
> +} while (!final);
>
> return 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] avformat/tests: add /imf to .gitignore
On 5/2/22 12:07, p...@sandflow.com wrote: From: Pierre-Anthony Lemieux --- libavformat/tests/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore index 7ceb7a356b..aabf76345e 100644 --- a/libavformat/tests/.gitignore +++ b/libavformat/tests/.gitignore @@ -1,4 +1,5 @@ /fifo_muxer +/imf /movenc /noproxy /rtmpdh lgtm, I'll apply this soon. ___ 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] Add libavformat/tests/imf to .gitignore
On Fri, Feb 4, 2022 at 6:03 PM Zane van Iperen wrote: > > > > > On 7/1/22 13:55, p...@sandflow.com wrote: > > From: Pierre-Anthony Lemieux > > > > Signed-off-by: Pierre-Anthony Lemieux > > --- > > libavformat/tests/.gitignore | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore > > index 7ceb7a356b..aabf76345e 100644 > > --- a/libavformat/tests/.gitignore > > +++ b/libavformat/tests/.gitignore > > @@ -1,4 +1,5 @@ > > /fifo_muxer > > +/imf > > /movenc > > /noproxy > > /rtmpdh > > Could you please change the subject something like: >avformat/tests: add /imf to .gitignore Done at http://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/292619.html > > > After that, lgtm. > ___ > 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".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Saturday, February 5, 2022 2:20 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 23:24:58 +, Soft Works wrote:
> > You want to "pollute" gazillions of subtitle streams in the
> > world from multiple subtitle formats with invisible
> > characters in order to solve an escaping problem in ffmpeg?
>
> I do not consider using characters that are explicitly recommended to
> be
> used by Unicode to be “polluting”. Further consider that as mentioned
> invisible characters in ASS are not uncommon anyway already and
> conversion
> from ASS to something else are rare due to being generally lossy.
> Lossy
> with regards to typesetting that is, removing breaking hints in form
> of
> plain Unicode characters would be a new form of lossyness.
>
> > [From the other mail:]
> > I'm not into changing ffmpeg's ass output, it's all
> > about the internally used ass format and the escaping is
> > a central problem there.
>
> I’m not interested in reworking ffmpeg’s internal subtitle handling.
> The proposed patch is a clear improvement over the status quo which
> is plain incorrect. Within reasonable effort and sound arguments for
> it adjustments to the patch can be made; reworking ffmpeg internals is
> imo not “reasonable” effort to correct an uncontestedly wrong escape.
>
> You have two options:
> Either finally tell me what I asked about:
> where (as in which file and function) removing wordjoiners should
> even happen and where possible lingering “\\ → \” conversions
> presumably
> are and if it’s simple enough I can add a removal accompanied by a
> comment
> pointing out that this can go wrong.
> Or go ahead and create your own patch.
>
> ~~
>
> > > > I'm not sure whether all ffmpeg text-sub encoders can handle
> > > > those chars - which could be verified of course.
> > >
> > > Since it's in the BMP and ffmpeg already seems happy to assume
> some
> > > UTF-8
> > > support by converting everything to it, I'm not worried about this
> > > until
> > > proven wrong.
> >
> > Proven wrong: https://github.com/libass/libass/issues/507
>
> This issue is not at all wordjoiner specific despite the name.
> As far as I recall this never lead to wrong rendering.
> With HarfBuzz, the only fully featured shaping backend of libass,
> control characters were and are handled by HarfBuzz.
> And even with FriBiDi U+2060 was ignored since long before (2012)
> the linked issue was opened.
>
> What that issue really is about is a combination of two more general
> issues. libass is currently not caching failure to lookup a glyph
> leading
> to multiple messages and at worst a perf degradation if no font on the
> font pool contained a glyph for a particular glyph. And the
> realisation
> that libass’ font-fallback strategy is not ideal for prefix-type
> control
> characters, characters which visibly affect both neighbours and a few
> others.
> The word-joiner is only highlighted here as due to its usage as an
> backslash escape its commonly passed to libass and a high enough
> percentage of fonts doesn’t contain it to create reports about it.
>
>
> For further reference: U+2060 was added in Unicode 3.2 released 2002.
> If you want to strip it because it might not render correctly you
> should
> also strip most emoji, the uppercase eszett ẞ and several actively
> used writing systems in their entirety.
Let's try to approach this from a different side. Which case is
your [1/2] commit actually supposed to fix?
How did you test your patch?
Can we please go over an example?
Thanks,
sw
___
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/tests: add /imf to .gitignore
From: Pierre-Anthony Lemieux --- libavformat/tests/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore index 7ceb7a356b..aabf76345e 100644 --- a/libavformat/tests/.gitignore +++ b/libavformat/tests/.gitignore @@ -1,4 +1,5 @@ /fifo_muxer +/imf /movenc /noproxy /rtmpdh -- 2.17.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] [PATCH] avformat/imf: s/++i/i++/g
On 4/2/22 17:58, Paul B Mahol wrote: Never apply this. Very sorry state of project. Iirc, there was some discussion on IRC about it. If you're okay with it, then I'll apply it. ___ 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] Add libavformat/tests/imf to .gitignore
On 7/1/22 13:55, p...@sandflow.com wrote: From: Pierre-Anthony Lemieux Signed-off-by: Pierre-Anthony Lemieux --- libavformat/tests/.gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore index 7ceb7a356b..aabf76345e 100644 --- a/libavformat/tests/.gitignore +++ b/libavformat/tests/.gitignore @@ -1,4 +1,5 @@ /fifo_muxer +/imf /movenc /noproxy /rtmpdh Could you please change the subject something like: avformat/tests: add /imf to .gitignore After that, lgtm. ___ 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 10/13] cbs_mpeg2.c: use a while loop with a loop condition instead of an infinite loop
Scott Theisen:
> This enhances the clarity of the code.
> ---
> libavcodec/cbs_mpeg2.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index d41477620e..afe78eef9a 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -148,7 +148,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> CodedBitstreamUnitType unit_type;
> uint32_t start_code = -1;
> size_t unit_size;
> -int err, i, final = 0;
> +int err, i = 0, final = 0;
>
> start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
> &start_code, 1);
> @@ -157,7 +157,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
> *ctx,
> return AVERROR_INVALIDDATA;
> }
>
> -for (i = 0;; i++) {
> +while (!final) {
> unit_type = start_code & 0xff;
>
> if (start == frag->data + frag->data_size) {
> @@ -190,10 +190,8 @@ static int
> cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
> if (err < 0)
> return err;
>
> -if (final)
> -break;
> -
> start = end;
> +i++;
> }
>
> return 0;
I disagree that this enhances clarity: When using a counter, a for loop
is the most natural.
- 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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
On Fri, Feb 04, 2022 at 23:24:58 +, Soft Works wrote: > You want to "pollute" gazillions of subtitle streams in the > world from multiple subtitle formats with invisible > characters in order to solve an escaping problem in ffmpeg? I do not consider using characters that are explicitly recommended to be used by Unicode to be “polluting”. Further consider that as mentioned invisible characters in ASS are not uncommon anyway already and conversion from ASS to something else are rare due to being generally lossy. Lossy with regards to typesetting that is, removing breaking hints in form of plain Unicode characters would be a new form of lossyness. > [From the other mail:] > I'm not into changing ffmpeg's ass output, it's all > about the internally used ass format and the escaping is > a central problem there. I’m not interested in reworking ffmpeg’s internal subtitle handling. The proposed patch is a clear improvement over the status quo which is plain incorrect. Within reasonable effort and sound arguments for it adjustments to the patch can be made; reworking ffmpeg internals is imo not “reasonable” effort to correct an uncontestedly wrong escape. You have two options: Either finally tell me what I asked about: where (as in which file and function) removing wordjoiners should even happen and where possible lingering “\\ → \” conversions presumably are and if it’s simple enough I can add a removal accompanied by a comment pointing out that this can go wrong. Or go ahead and create your own patch. ~~ > > > I'm not sure whether all ffmpeg text-sub encoders can handle > > > those chars - which could be verified of course. > > > > Since it's in the BMP and ffmpeg already seems happy to assume some > > UTF-8 > > support by converting everything to it, I'm not worried about this > > until > > proven wrong. > > Proven wrong: https://github.com/libass/libass/issues/507 This issue is not at all wordjoiner specific despite the name. As far as I recall this never lead to wrong rendering. With HarfBuzz, the only fully featured shaping backend of libass, control characters were and are handled by HarfBuzz. And even with FriBiDi U+2060 was ignored since long before (2012) the linked issue was opened. What that issue really is about is a combination of two more general issues. libass is currently not caching failure to lookup a glyph leading to multiple messages and at worst a perf degradation if no font on the font pool contained a glyph for a particular glyph. And the realisation that libass’ font-fallback strategy is not ideal for prefix-type control characters, characters which visibly affect both neighbours and a few others. The word-joiner is only highlighted here as due to its usage as an backslash escape its commonly passed to libass and a high enough percentage of fonts doesn’t contain it to create reports about it. For further reference: U+2060 was added in Unicode 3.2 released 2002. If you want to strip it because it might not render correctly you should also strip most emoji, the uppercase eszett ẞ and several actively used writing systems in their entirety. ___ 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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Friday, February 4, 2022 10:52 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 02:30:37 +0100, Andreas Rheinhardt wrote:
> > All text-based subtitles are supposed to be UTF-8 when they reach
> the
> > decoder; if it isn't, the user has to set the appropriate -
> sub_charenc
> > and -sub_charenc_mode.
> >
> > - Andreas
>
> Thanks for the info! Then at least the UTF-8 assumption
> is no problem after all.
>
>
[..]
> >
> > I'm not sure whether all ffmpeg text-sub encoders can handle
> > those chars - which could be verified of course.
>
> Since it's in the BMP and ffmpeg already seems happy to assume some
> UTF-8
> support by converting everything to it, I'm not worried about this
> until
> proven wrong.
Proven wrong: https://github.com/libass/libass/issues/507
> > Finally, those chars are a pest. I'm using them myself for a
> > specific use case, but when you don't know they are there, it can
> > drive you totally mad, eventually even thinking your system or
> > software is faulty.
> >
> > Example:
> >
> > Open your patch file [2/2] and search for the string
> > "123456\NAscending". You can see the string in two lines, but search
> > will only find one of them.
> >
> > Or just look at the two lines directly. They are preceded by + and -
> > even though both appear identical.
>
> Actually, I see this with helpful colouring lost here:
>
> -Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending:
> 123456\NAscending: 123456^M
> +Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending:
> <200f>123456<200e>\NAscending: 123456^M
I didn't say you won't be a able to find a viewer that can display
them. :-)
> More plain-text oriented editors likely won't show them though, yes.
Yes => pest
> > That might be true, but I think it's valid to say that such
> characters
> > are very unusual "original" subtitle sources and that's why I don't
> > think it's a good idea for ffmpeg to start injecting them.
>
> Don't underestimate what subtitle authors can come up with :)
Sure. But a subtitle author is responsible for their authored
subtitles while ffmpeg is responsible for encoding of large
part of the world's subtitles.
And from that same perspective I find the relation of this
proposal somewhat insane:
You want to "pollute" gazillions of subtitle streams in the
world from multiple subtitle formats with invisible
characters in order to solve an escaping problem in ffmpeg?
softworkz
___
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] Correctly set low_delay_hrd_flag when rewriting fixed_frame_rate_flag in h264_metadata bitstream filter
Rereading the spec, I think it only makes sense to update the
low_delay_hrd_flag parameter when the bitstream filter option for
fixed_frame_rate_flag is 1 to avoid clobbering the existing value in
other cases. I tried to work through all the scenarios below:
/ (BSF options) -> Resulting flags:
0/0 (tick_rate=10) -> 0/0
0/1 (tick_rate=10) -> 0/1
1/0 (tick_rate=10) -> 1/0
1/1 (tick_rate=10) -> 1/1
0/0 (fixed_frame_rate_flag=0) -> 0/1
0/1 (fixed_frame_rate_flag=0) -> 0/1
1/0 (fixed_frame_rate_flag=0) -> 0/0
1/1 (fixed_frame_rate_flag=0) -> 0/1
0/0 (fixed_frame_rate_flag=1) -> 1/0
0/1 (fixed_frame_rate_flag=1) -> 1/0
1/0 (fixed_frame_rate_flag=1) -> 1/0
1/1 (fixed_frame_rate_flag=1) -> 1/0
Signed-off-by: Keshav Varma
---
libavcodec/h264_metadata_bsf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 9df99cbae3..3e558a5166 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -228,7 +228,13 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
sps->vui.timing_info_present_flag = 1;
need_vui = 1;
}
+
+// Set fixed frame rate flag and update low_delay_hrd_flag to match
SET_VUI_FIELD(fixed_frame_rate_flag);
+if (ctx->fixed_frame_rate_flag) {
+sps->vui.low_delay_hrd_flag = 0;
+}
+
if (ctx->zero_new_constraint_set_flags) {
sps->constraint_set4_flag = 0;
sps->constraint_set5_flag = 0;
--
2.31.1
On Fri, Feb 4, 2022 at 3:00 PM Keshav Varma wrote:
>
> After changes like ef13faf, the h264_metadata bitstream filter stopped
> working when using the fixed_frame_rate_flag option on an input stream
> that doesn't contain VUI because the default inferred value of
> low_hrd_delay_flag seems to be 1. ffmpeg used to raise a warning, but
> proceeded anyway but now aborts after the other fixes since the output
> is rightfully invalid.
>
> I believe this change makes the bitstream filter conform to page 403
> of the ITU spec: "When fixed_frame_rate_flag is equal to 1,
> low_delay_hrd_flag shall be equal to 0. When low_delay_hrd_flag is not
> present, its value shall be inferred to be equal to 1 −
> fixed_frame_rate_flag."
>
> Signed-off-by: Keshav Varma
> ---
> libavcodec/h264_metadata_bsf.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
> index 9df99cbae3..59a7eba546 100644
> --- a/libavcodec/h264_metadata_bsf.c
> +++ b/libavcodec/h264_metadata_bsf.c
> @@ -228,7 +228,13 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
> sps->vui.timing_info_present_flag = 1;
> need_vui = 1;
> }
> -SET_VUI_FIELD(fixed_frame_rate_flag);
> +
> +// Set fixed frame rate flag and update low_delay_hrd_flag to match
> +if (ctx->fixed_frame_rate_flag >= 0) {
> +sps->vui.fixed_frame_rate_flag = ctx->fixed_frame_rate_flag;
> +sps->vui.low_delay_hrd_flag = 1 - sps->vui.fixed_frame_rate_flag;
> +}
> +
> if (ctx->zero_new_constraint_set_flags) {
> sps->constraint_set4_flag = 0;
> sps->constraint_set5_flag = 0;
> --
> 2.31.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".
[FFmpeg-devel] [PATCH] Correctly set low_delay_hrd_flag when rewriting fixed_frame_rate_flag in h264_metadata bitstream filter
After changes like ef13faf, the h264_metadata bitstream filter stopped
working when using the fixed_frame_rate_flag option on an input stream
that doesn't contain VUI because the default inferred value of
low_hrd_delay_flag seems to be 1. ffmpeg used to raise a warning, but
proceeded anyway but now aborts after the other fixes since the output
is rightfully invalid.
I believe this change makes the bitstream filter conform to page 403
of the ITU spec: "When fixed_frame_rate_flag is equal to 1,
low_delay_hrd_flag shall be equal to 0. When low_delay_hrd_flag is not
present, its value shall be inferred to be equal to 1 −
fixed_frame_rate_flag."
Signed-off-by: Keshav Varma
---
libavcodec/h264_metadata_bsf.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
index 9df99cbae3..59a7eba546 100644
--- a/libavcodec/h264_metadata_bsf.c
+++ b/libavcodec/h264_metadata_bsf.c
@@ -228,7 +228,13 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
sps->vui.timing_info_present_flag = 1;
need_vui = 1;
}
-SET_VUI_FIELD(fixed_frame_rate_flag);
+
+// Set fixed frame rate flag and update low_delay_hrd_flag to match
+if (ctx->fixed_frame_rate_flag >= 0) {
+sps->vui.fixed_frame_rate_flag = ctx->fixed_frame_rate_flag;
+sps->vui.low_delay_hrd_flag = 1 - sps->vui.fixed_frame_rate_flag;
+}
+
if (ctx->zero_new_constraint_set_flags) {
sps->constraint_set4_flag = 0;
sps->constraint_set5_flag = 0;
--
2.31.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] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Friday, February 4, 2022 10:19 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 06:48:40 +, Soft Works wrote:
> > > [two-part message removed for brevity]
> >
> > I've found out where the \{ and \} escaping has come from: libass
>
> As already written in the commit-message of the first patch..
>
>
> You already noticed your proposal only works with VSFilters,
> but even without this it's a terrible approach. Consider:
> - fullwidth characters have different metrics then the "regular" ones
> - fullwidth and small characters have a different visual appearance
> - support for fullwidth and small characters in fonts is much rarer
>than support for plain {}
> - fullwidth characters are commonly used _as fullwidth characters_
>e.g. in text using one of the CJK writing systems.
>Replacing them with non-fullwidth variants when transforming
>away from ASS is guaranteed to be disastrous.
> - Not sure if applies, but something to keep in mind:
>{\r} is not a noop if the source-format had any sort of per-event
>styling which got prepended to the ASS event text before
>using the plain-text conversion for the rest of the event.
No, this doesn't apply from what I've seen, but {} might
still be preferable.
> As noted in the discussion of the libass issue you linked,
> it’s not unusual for ASS subtitle authors to employ
> fullwidth curly braces for displaying curly braces
> in all ASS-renderers. However, they have tight control over the
> fonts used and can carefully select them to match the visual
> appearance and compensate differing metrics with bespoke
> local adjustments to \fs and negative \fsp.
> ffmpeg does not have tight control over the fonts and it'd be silly
> to require users to pass in special fonts just to render curly braces.
I basically agree to everything you say - except that there's a
little misunderstanding. Maybe I haven't explained well enough.
We use ASS as the internal format to which all text subtitles are
decoded and from which all text subtitles are encoded for output
and for the upcoming subtitle filtering it's also the one and only
format for text subtitles.
BUT: That does not necessarily mean that the internally used
ASS must be exactly the same that we're outputting when encoding
to ASS. And that's why we need to consider this as two separate
steps. It would also be possible to have options at the ass encoder
to control the compatibility of the encoded ASS output.
That internal ASS format already has some quirks that some had
introduced in order to achieve certain things when other subtitle
formats are involved at the input and at the output. That's why
we should not continue adding one workaround on top of another
but try to clean those things up instead.
With your submission, you are actually pointing at a core point
of evil: the escaping of braces in combination with the backslash
logic introduces an unresolvable ambiguity. And when we don't
clean that up, it won't be possible to get on a sane path.
> If you want to make the rendering in VSFilters not complettly broken,
> try to do what the libass-wiki recommends and add an empty command
> block after an escaped opening curly brace. This way VSFilters
> will display a lone backslash instead of a opening curly brace
> but otherwise work fine without swallowing up text.
> If done consistently closing curly braces won't need to be
> escaped at all anymore.
> However, such a VSFilter-compatibility change is unrelated to
> fixing the broken \\ escape which doesn't work anywhere.
see above, I'm not into changing ffmpeg's ass output, it's all
about the internally used ass format and the escaping is
a central problem there.
> (Note that the wordjoiner doesn't have font or spacing issues as
> it’s defined to be invisible and zero-width.
Yes, and the use of this has already created issues, even in libass:
https://github.com/libass/libass/issues/507
So it's very likely to cause issues in other implementations
as well, and not many are developed as actively as libass.
(and even that doesn't help when you don't get an update
for your device/software).
I wouldn't want to close like that, but I'm getting distracted
right now. Will get back later.
softworkz
___
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] avutil: use getauxval on linux/android for CPU capabilities
On Fri, 4 Feb 2022, Aman Karmani wrote: From: Aman Karmani fixes #6578 Signed-off-by: Aman Karmani --- libavutil/arm/cpu.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) It would be good if the commit message actually explained the upsides to doing this. It's also important to point out that this function didn't use to exist in all Android versions (hence the /proc/cpuinfo parsing). The point when the function appeared (Android 4.4, API 20, according to the referenced trac issue) is kinda far in the past today, but I'm not sure if strictly all users of the library have stopped supporting older versions still - at least for some use cases I've heard of recently, Android 4.1 was still supported. If coupled with a configure check for the function, so that users explicitly targeting an older version that lack it wouldn't get it, I guess this could be more easily acceptable. // 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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
On Fri, Feb 04, 2022 at 02:30:37 +0100, Andreas Rheinhardt wrote: > All text-based subtitles are supposed to be UTF-8 when they reach the > decoder; if it isn't, the user has to set the appropriate -sub_charenc > and -sub_charenc_mode. > > - Andreas Thanks for the info! Then at least the UTF-8 assumption is no problem after all. On Fri, Feb 04, 2022 at 01:57:48 +, Soft Works wrote: > > There's no way of knowing whether the word-joiner comes from > > a conversion performed by ffmpeg in the past or already existed > > in the original source. > > That might be true, but I think it's valid to say that such characters > are very unusual "original" subtitle sources and that's why I don't > think it's a good idea for ffmpeg to start injecting them. Don't underestimate what subtitle authors can come up with :) > Subtitle implementations are often rather minimal, especially in > hardware devices and might not always cover the full range of > UTF-8 specifics. The wordjoiner lies in the Basic Multilingual Plane, so even ancient UTF-8 implementations assuming all of Unicode’s codepoints fit in 16bits (i.e. 3-bytes max per codepoint in UTF-8) will be able to understand it. > > However, the wordjoiner does not alter the visually appearance and > > is unlikely to change line-breaking properties; that's why I chose > > a word-joiner. Therefore I don't think removing (only) the inserted > > word-joiners is possible, > > Why not? As it seems to be required for ASS encoding only, all other > output formats should remain unaffected. Because — as written before — it can exist in the original source. Unicode recommends using the wordjoiner eg to prevent linebreaks between two characters without any additional side-effects as eg the combining-grapheme-joiner would cause. > > but also not necessary. > > I'm not sure whether all ffmpeg text-sub encoders can handle > those chars - which could be verified of course. Since it's in the BMP and ffmpeg already seems happy to assume some UTF-8 support by converting everything to it, I'm not worried about this until proven wrong. > Finally, those chars are a pest. I'm using them myself for a > specific use case, but when you don't know they are there, it can > drive you totally mad, eventually even thinking your system or > software is faulty. > > Example: > > Open your patch file [2/2] and search for the string > "123456\NAscending". You can see the string in two lines, but search > will only find one of them. > > Or just look at the two lines directly. They are preceded by + and - > even though both appear identical. Actually, I see this with helpful colouring lost here: -Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: 123456\NAscending: 123456^M +Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending: <200f>123456<200e>\NAscending: 123456^M More plain-text oriented editors likely won't show them though, yes. On this topic, finding raw bidi-marks in ASS subtitles for RTL-languages is not that unusual though, to give an example for "invisible characters" being used manually in the original source. (Because VSFilters (and libass in the interest of compatibility) assumes LTR by default and other things) Even if I thought removing all wordjoiners when converting from ASS was a good idea, I still wouldn't know where to do this (or where to look to remove possibly lingering attempts to recollapse \\ into \). ___ 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 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
On Fri, Feb 04, 2022 at 06:48:40 +, Soft Works wrote:
> > [two-part message removed for brevity]
>
> I've found out where the \{ and \} escaping has come from: libass
As already written in the commit-message of the first patch..
You already noticed your proposal only works with VSFilters,
but even without this it's a terrible approach. Consider:
- fullwidth characters have different metrics then the "regular" ones
- fullwidth and small characters have a different visual appearance
- support for fullwidth and small characters in fonts is much rarer
than support for plain {}
- fullwidth characters are commonly used _as fullwidth characters_
e.g. in text using one of the CJK writing systems.
Replacing them with non-fullwidth variants when transforming
away from ASS is guaranteed to be disastrous.
- Not sure if applies, but something to keep in mind:
{\r} is not a noop if the source-format had any sort of per-event
styling which got prepended to the ASS event text before
using the plain-text conversion for the rest of the event.
As noted in the discussion of the libass issue you linked,
it’s not unusual for ASS subtitle authors to employ
fullwidth curly braces for displaying curly braces
in all ASS-renderers. However, they have tight control over the
fonts used and can carefully select them to match the visual
appearance and compensate differing metrics with bespoke
local adjustments to \fs and negative \fsp.
ffmpeg does not have tight control over the fonts and it'd be silly
to require users to pass in special fonts just to render curly braces.
If you want to make the rendering in VSFilters not complettly broken,
try to do what the libass-wiki recommends and add an empty command
block after an escaped opening curly brace. This way VSFilters
will display a lone backslash instead of a opening curly brace
but otherwise work fine without swallowing up text.
If done consistently closing curly braces won't need to be
escaped at all anymore.
However, such a VSFilter-compatibility change is unrelated to
fixing the broken \\ escape which doesn't work anywhere.
(Note that the wordjoiner doesn't have font or spacing issues as
it’s defined to be invisible and zero-width.
If a user supplies a special font like FontoXMLWhitespace¹
showing the word-joiner that's not a problem anymore but a
deliberate choice)
[1]:
https://documentation.fontoxml.com/latest/whitespace-visualization-5b5c6b154c78#id-18b2211e-79cb-4b85-8390-1e54d0f45466
___
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] avutil: use getauxval on linux/android for CPU capabilities
From: Aman Karmani
fixes #6578
Signed-off-by: Aman Karmani
---
libavutil/arm/cpu.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/libavutil/arm/cpu.c b/libavutil/arm/cpu.c
index 81e85e2525..983e5a4d83 100644
--- a/libavutil/arm/cpu.c
+++ b/libavutil/arm/cpu.c
@@ -36,6 +36,7 @@
#include
#include
#include
+#include
#include "libavutil/avstring.h"
#define AT_HWCAP16
@@ -48,6 +49,15 @@
#define HWCAP_VFPv3 (1 << 13)
#define HWCAP_TLS (1 << 15)
+static int get_auxval(uint32_t *hwcap)
+{
+unsigned long ret = getauxval(AT_HWCAP);
+if (ret == 0)
+return -1;
+*hwcap = ret;
+return 0;
+}
+
static int get_hwcap(uint32_t *hwcap)
{
struct { uint32_t a_type; uint32_t a_val; } auxv;
@@ -106,9 +116,10 @@ int ff_get_cpu_flags_arm(void)
int flags = CORE_CPU_FLAGS;
uint32_t hwcap;
-if (get_hwcap(&hwcap) < 0)
-if (get_cpuinfo(&hwcap) < 0)
-return flags;
+if (get_auxval(&hwcap) < 0)
+if (get_hwcap(&hwcap) < 0)
+if (get_cpuinfo(&hwcap) < 0)
+return flags;
#define check_cap(cap, flag) do { \
if (hwcap & HWCAP_ ## cap) \
--
2.33.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 6/7] avcodec/internal.h: Move avpriv_find_start_code() to startcode.h
On 2/4/22 10:16, Andreas Rheinhardt wrote:
This is by definition the appropriate place for it.
Remove all the now unnecessary libavcodec/internal.h inclusions;
also remove other unnecessary headers from the affected files.
Signed-off-by: Andreas Rheinhardt
---
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index b19befef21..c7c7323351 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -27,7 +27,6 @@
#include "config.h"
#include "libavutil/avassert.h"
-#include "libavutil/avstring.h"
#include "libavutil/channel_layout.h"
#include "libavutil/intreadwrite.h"
#include "libavutil/mem.h"
@@ -40,12 +39,9 @@
#include "thread.h"
#include "internal.h"
#include "put_bits.h"
-#include "raw.h"
+#include "startcode.h"
#include
-#include
-#include
#include
-#include
void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t min_size)
{
Shouldn't you also move the definition to startcode.c as well? Then
utils.c doesn't need startcode.h.
___
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 4/7] avcodec/cbs_mpeg2: Simplify splitting fragment
On 2/4/22 10:16, Andreas Rheinhardt wrote:
avpriv_find_start_code() supports non-contiguous buffers
by maintaining a state that allows to find start codes
that span across multiple buffers; a consequence thereof
is that avpriv_find_start_code() is given a zero-sized
buffer, it does not modify this state, so that it appears
as if a start code was found if the state contained a start code.
This can e.g. happen with Sequence End units in MPEG-2 and
to counter this, cbs_mpeg2_split_fragment() reset the state
when it has already encountered the end of the fragment
in order to add the last unit (if it is only of the form 00 00 01 xy)
only once; it also used a flag to set whether this is the final unit.
Yet this can be improved by simply resetting state unconditionally
(thereby avoiding a branch); the flag can be removed by just checking
whether we have a valid start code (of the next unit to add)
at the end.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 2bf38c6001..90d667ddc8 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -149,7 +149,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
uint32_t start_code = -1;
size_t unit_size;
int err;
-int final = 0;
start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
&start_code);
@@ -161,14 +160,11 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
do {
unit_type = start_code & 0xff;
-if (start == frag->data + frag->data_size) {
-// The last four bytes form a start code which constitutes
-// a unit of its own. In this situation avpriv_find_start_code
-// won't modify start_code at all so modify start_code so that
-// the next unit will be treated as the last unit.
-start_code = 0;
-}
-
+// Reset start_code to ensure that avpriv_find_start_code()
+// really reads a new start code and does not reuse the old
+// start code in any way (as e.g. happens when there is a
+// Sequence End unit at the very end of a packet).
+start_code = UINT32_MAX;
end = avpriv_find_start_code(start--, frag->data + frag->data_size,
&start_code);
@@ -183,7 +179,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext *ctx,
} else {
// We didn't find a start code, so this is the final unit.
unit_size = end - start;
- final = 1;
}
err = ff_cbs_append_unit_data(frag, unit_type, (uint8_t*)start,
@@ -192,7 +187,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
return err;
start = end;
-} while (!final);
+} while ((start_code >> 8) == 0x01);
return 0;
}
I assume this is inspired by my patch series. Did you see v2 of my
patch series where I made similar and more extensive clarifications?
The check for sequence_end_codes should not be in the loop. It harms
readability, although you also eliminated the branch.
Regards,
Scott
___
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/imf: s/++i/i++/g
On Thu, Feb 3, 2022 at 11:58 PM Paul B Mahol wrote: > > Never apply this. I am not a maintainer and so cannot apply the patch. There is another minor patch that has also been outstanding for some time now: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=5700 > Very sorry state of project. Can you be more specific? Are you referring to code style issues? If so, there is a draft patch at [1]. [1] https://github.com/sandflow/ffmpeg-imf/pull/85 I have been reluctant to propose it until: (a) I am more confident in the expected coding style; and (b) issues logged in trac are fixed. Regarding (a), I asked the following question, which I never received feedback on: http://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/290990.html This prompted me to work on expanding the code style documentation, which I mentioned on IRC and plan to proposed shortly: https://github.com/sandflow/ffmpeg-imf/pull/95 Regarding (b), the following patch is up for review: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=5873 I can prioritize (a) if folks feel this is the right approach. There is also an outstanding proposal [2] to refactor uuid functionality. Have you had a chance to review? [2] http://ffmpeg.org/pipermail/ffmpeg-devel/2022-January/291917.html > ___ > 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".
Re: [FFmpeg-devel] [PATCH] zscale video filter performance optimization 4x
Is moving all this code really needed? Note that width/height can change between frames so that is supported by scale filter. With your change that is not more possible? ___ 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] avdevice/avfoundation: check strdup
---
libavdevice/avfoundation.m | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 2078c4879c..8f5e2bd120 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -310,18 +310,21 @@ static void destroy_context(AVFContext* ctx)
}
}
-static void parse_device_name(AVFormatContext *s)
+static int parse_device_name(AVFormatContext *s)
{
AVFContext *ctx = (AVFContext*)s->priv_data;
ctx->url = av_strdup(s->url);
char *save;
+if (!ctx->url)
+return AVERROR(ENOMEM);
if (ctx->url[0] != ':') {
ctx->video_filename = av_strtok(ctx->url, ":", &save);
ctx->audio_filename = av_strtok(NULL, ":", &save);
} else {
ctx->audio_filename = av_strtok(ctx->url, ":", &save);
}
+return 0;
}
/**
@@ -760,6 +763,7 @@ static int get_audio_config(AVFormatContext *s)
static int avf_read_header(AVFormatContext *s)
{
+int ret = 0;
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
uint32_t num_screens= 0;
AVFContext *ctx = (AVFContext*)s->priv_data;
@@ -812,7 +816,9 @@ static int avf_read_header(AVFormatContext *s)
}
// parse input filename for video and audio device
-parse_device_name(s);
+ret = parse_device_name(s);
+if (ret)
+goto fail;
// check for device index given in filename
if (ctx->video_device_index == -1 && ctx->video_filename) {
@@ -1002,6 +1008,8 @@ static int avf_read_header(AVFormatContext *s)
fail:
[pool release];
destroy_context(ctx);
+if (ret)
+return ret;
return AVERROR(EIO);
}
--
2.31.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".
[FFmpeg-devel] [PATCH 1/2] avdevice/avfoundation: fix memleak
---
libavdevice/avfoundation.m | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/libavdevice/avfoundation.m b/libavdevice/avfoundation.m
index 0cd6e646d5..2078c4879c 100644
--- a/libavdevice/avfoundation.m
+++ b/libavdevice/avfoundation.m
@@ -106,6 +106,7 @@ typedef struct
int audio_device_index;
int audio_stream_index;
+char*url;
char*video_filename;
char*audio_filename;
@@ -299,6 +300,7 @@ static void destroy_context(AVFContext* ctx)
ctx->avf_delegate= NULL;
ctx->avf_audio_delegate = NULL;
+av_freep(&ctx->url);
av_freep(&ctx->audio_buffer);
pthread_mutex_destroy(&ctx->frame_lock);
@@ -311,14 +313,14 @@ static void destroy_context(AVFContext* ctx)
static void parse_device_name(AVFormatContext *s)
{
AVFContext *ctx = (AVFContext*)s->priv_data;
-char *tmp = av_strdup(s->url);
+ctx->url = av_strdup(s->url);
char *save;
-if (tmp[0] != ':') {
-ctx->video_filename = av_strtok(tmp, ":", &save);
+if (ctx->url[0] != ':') {
+ctx->video_filename = av_strtok(ctx->url, ":", &save);
ctx->audio_filename = av_strtok(NULL, ":", &save);
} else {
-ctx->audio_filename = av_strtok(tmp, ":", &save);
+ctx->audio_filename = av_strtok(ctx->url, ":", &save);
}
}
--
2.31.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".
[FFmpeg-devel] [PATCH 7/7] avcodec/cbs_mpeg2: Use smaller scope for variables
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 8d45d1ceeb..dfea12ef1b 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -144,10 +144,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
CodedBitstreamFragment *frag,
int header)
{
-const uint8_t *start, *end;
-CodedBitstreamUnitType unit_type;
+const uint8_t *start;
uint32_t start_code = -1;
-size_t unit_size;
int err;
start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
@@ -158,7 +156,9 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
}
do {
-unit_type = start_code & 0xff;
+CodedBitstreamUnitType unit_type = start_code & 0xff;
+const uint8_t *end;
+size_t unit_size;
// Reset start_code to ensure that avpriv_find_start_code()
// really reads a new start code and does not reuse the old
--
2.32.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 6/7] avcodec/internal.h: Move avpriv_find_start_code() to startcode.h
This is by definition the appropriate place for it.
Remove all the now unnecessary libavcodec/internal.h inclusions;
also remove other unnecessary headers from the affected files.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cavsdec.c | 1 +
libavcodec/cbs_mpeg2.c| 2 +-
libavcodec/extract_extradata_bsf.c| 3 +--
libavcodec/h264_parser.c | 3 +--
libavcodec/internal.h | 4
libavcodec/mpeg12.c | 6 +-
libavcodec/mpeg12dec.c| 4 +---
libavcodec/mpeg4_unpack_bframes_bsf.c | 2 +-
libavcodec/mpegvideo_parser.c | 2 +-
libavcodec/remove_extradata_bsf.c | 2 +-
libavcodec/startcode.h| 4
libavcodec/utils.c| 6 +-
libavcodec/vc1_common.h | 2 +-
libavformat/avidec.c | 2 +-
libavformat/avs2dec.c | 2 +-
libavformat/avs3dec.c | 2 +-
libavformat/cavsvideodec.c| 2 +-
libavformat/mpegtsenc.c | 2 +-
libavformat/mpegvideodec.c| 2 +-
libavformat/mxfenc.c | 6 ++
libavformat/rtpenc_mpv.c | 2 +-
21 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 692c77eb39..894aa1b54a 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -32,6 +32,7 @@
#include "cavs.h"
#include "internal.h"
#include "mpeg12data.h"
+#include "startcode.h"
static const uint8_t mv_scan[4] = {
MV_FWD_X0, MV_FWD_X1,
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 90d667ddc8..8d45d1ceeb 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -21,7 +21,7 @@
#include "cbs.h"
#include "cbs_internal.h"
#include "cbs_mpeg2.h"
-#include "internal.h"
+#include "startcode.h"
#define HEADER(name) do { \
diff --git a/libavcodec/extract_extradata_bsf.c
b/libavcodec/extract_extradata_bsf.c
index dbcb8508b0..027a578af1 100644
--- a/libavcodec/extract_extradata_bsf.c
+++ b/libavcodec/extract_extradata_bsf.c
@@ -18,8 +18,6 @@
#include
-#include "libavutil/common.h"
-#include "libavutil/intreadwrite.h"
#include "libavutil/log.h"
#include "libavutil/opt.h"
@@ -31,6 +29,7 @@
#include "h2645_parse.h"
#include "h264.h"
#include "hevc.h"
+#include "startcode.h"
#include "vc1_common.h"
typedef struct ExtractExtradataContext {
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 4002bcad77..50810f1789 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -27,7 +27,6 @@
#define UNCHECKED_BITSTREAM_READER 1
-#include
#include
#include "libavutil/avutil.h"
@@ -46,9 +45,9 @@
#include "h264_ps.h"
#include "h2645_parse.h"
#include "h264data.h"
-#include "internal.h"
#include "mpegutils.h"
#include "parser.h"
+#include "startcode.h"
typedef struct H264ParseContext {
ParseContext pc;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 72ca1553f6..57a1c6206f 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -285,10 +285,6 @@ int ff_thread_can_start_frame(AVCodecContext *avctx);
int avpriv_h264_has_num_reorder_frames(AVCodecContext *avctx);
-const uint8_t *avpriv_find_start_code(const uint8_t *p,
- const uint8_t *end,
- uint32_t *state);
-
int avpriv_codec_get_cap_skip_frame_fill_param(const AVCodec *codec);
/**
diff --git a/libavcodec/mpeg12.c b/libavcodec/mpeg12.c
index 58e03c05d4..5520960b74 100644
--- a/libavcodec/mpeg12.c
+++ b/libavcodec/mpeg12.c
@@ -29,18 +29,14 @@
#include "libavutil/attributes.h"
#include "libavutil/avassert.h"
-#include "libavutil/timecode.h"
#include "libavutil/thread.h"
-#include "internal.h"
#include "avcodec.h"
#include "mpegvideo.h"
-#include "error_resilience.h"
#include "mpeg12.h"
#include "mpeg12data.h"
#include "mpegvideodata.h"
-#include "bytestream.h"
-#include "thread.h"
+#include "startcode.h"
static const uint8_t table_mb_ptype[7][2] = {
{ 3, 5 }, // 0x01 MB_INTRA
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 4a7bd6d466..860e86aa74 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -34,14 +34,11 @@
#include "libavutil/mem_internal.h"
#include "libavutil/stereo3d.h"
#include "libavutil/timecode.h"
-#include "libavutil/video_enc_params.h"
#include "avcodec.h"
-#include "bytestream.h"
#include "error_resilience.h"
#include "hwconfig.h"
#include "idctdsp.h"
-#include "internal.h"
#include "mpeg_er.h"
#include "mpeg12.h"
#include "mpeg12data.h"
@@ -49,6 +46,7 @@
#include "mpegvideo.h"
#include "mpegvideodata.h"
#include "profiles.h"
+#include "startcode.h"
#include "thread.h"
#include "xvmc_internal.h"
diff --git a/libavcodec/mpeg4_unpack_bframes_bsf.c
b/libavcodec/mpeg4_unpack_bframes_bsf.c
index 6f8595713d..ae2c129d88 100644
--- a/libavcodec/mpeg4_unpack_bframes_
[FFmpeg-devel] [PATCH 5/7] all: Remove unnecessary libavcodec/internal.h inclusions
Signed-off-by: Andreas Rheinhardt
---
libavcodec/aac_adtstoasc_bsf.c | 1 -
libavcodec/aliaspixenc.c | 1 -
libavcodec/av1_parser.c| 1 -
libavcodec/avpacket.c | 6 ++
libavcodec/cbs_av1.c | 2 +-
libavcodec/cbs_vp9.c | 1 -
libavcodec/cljrenc.c | 2 --
libavcodec/dca_core.c | 1 +
libavcodec/dca_core.h | 2 --
libavcodec/dca_lbr.c | 1 +
libavcodec/dca_lbr.h | 2 --
libavcodec/dca_xll.c | 1 +
libavcodec/dca_xll.h | 2 --
libavcodec/dcadec.c| 1 +
libavcodec/dv.c| 7 ---
libavcodec/dvbsub_parser.c | 1 -
libavcodec/dxtory.c| 1 -
libavcodec/ffv1.c | 6 --
libavcodec/ffv1.h | 4
libavcodec/fitsenc.c | 1 -
libavcodec/flacdata.c | 2 +-
libavcodec/flacdata.h | 2 +-
libavcodec/h264_ps.c | 1 -
libavcodec/h264_refs.c | 1 -
libavcodec/hevc_parser.c | 1 -
libavcodec/hevc_refs.c | 2 --
libavcodec/hevcdec.c | 1 +
libavcodec/hevcdec.h | 2 --
libavcodec/ituh263dec.c| 1 -
libavcodec/jpeg2000dwt.c | 1 -
libavcodec/motion_est.c| 1 -
libavcodec/opusenc.h | 1 -
libavcodec/parser.c| 1 -
libavcodec/ratecontrol.c | 1 -
libavcodec/scpr.h | 5 -
libavcodec/sheervideo.c| 5 -
libavcodec/ttadata.c | 1 +
libavcodec/ttadata.h | 2 +-
libavcodec/twinvq.h| 1 -
libavcodec/vaapi_mpeg2.c | 1 -
libavcodec/vaapi_mpeg4.c | 1 -
libavcodec/vaapi_vc1.c | 1 -
libavcodec/vc1_parser.c| 1 -
libavcodec/vdpau_h264.c| 1 -
libavcodec/vdpau_hevc.c| 1 -
libavcodec/vdpau_vp9.c | 1 -
libavcodec/vp9mvs.c| 1 -
libavcodec/wma.c | 1 -
libavcodec/xbmenc.c| 1 -
libavcodec/xfaceenc.c | 1 -
libavcodec/xwdenc.c| 2 --
libavcodec/yuv4enc.c | 1 -
libavformat/assdec.c | 1 -
libavformat/h264dec.c | 1 -
libavformat/hcom.c | 1 -
libavformat/ipudec.c | 1 -
libavformat/jacosubdec.c | 1 -
libavformat/network.c | 1 -
libavformat/samidec.c | 2 --
libavformat/subviewerdec.c | 1 -
60 files changed, 12 insertions(+), 87 deletions(-)
diff --git a/libavcodec/aac_adtstoasc_bsf.c b/libavcodec/aac_adtstoasc_bsf.c
index 69453c706f..267ef97572 100644
--- a/libavcodec/aac_adtstoasc_bsf.c
+++ b/libavcodec/aac_adtstoasc_bsf.c
@@ -26,7 +26,6 @@
#include "put_bits.h"
#include "get_bits.h"
#include "mpeg4audio.h"
-#include "internal.h"
typedef struct AACBSFContext {
int first_frame_done;
diff --git a/libavcodec/aliaspixenc.c b/libavcodec/aliaspixenc.c
index fa273df9c2..27b7d1d8de 100644
--- a/libavcodec/aliaspixenc.c
+++ b/libavcodec/aliaspixenc.c
@@ -24,7 +24,6 @@
#include "avcodec.h"
#include "bytestream.h"
#include "encode.h"
-#include "internal.h"
#define ALIAS_HEADER_SIZE 10
diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c
index d2dfdb3580..4cbd7408a0 100644
--- a/libavcodec/av1_parser.c
+++ b/libavcodec/av1_parser.c
@@ -23,7 +23,6 @@
#include "libavutil/avassert.h"
#include "cbs.h"
#include "cbs_av1.h"
-#include "internal.h"
#include "parser.h"
typedef struct AV1ParseContext {
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 4f7a6b255c..bcbc4166cb 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -22,14 +22,12 @@
#include
#include "libavutil/avassert.h"
-#include "libavutil/common.h"
-#include "libavutil/internal.h"
+#include "libavutil/intreadwrite.h"
#include "libavutil/mathematics.h"
#include "libavutil/mem.h"
#include "libavutil/rational.h"
-#include "bytestream.h"
-#include "internal.h"
+#include "defs.h"
#include "packet.h"
#include "packet_internal.h"
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 04314443de..1229480567 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -20,10 +20,10 @@
#include "libavutil/opt.h"
#include "libavutil/pixfmt.h"
+#include "avcodec.h"
#include "cbs.h"
#include "cbs_internal.h"
#include "cbs_av1.h"
-#include "internal.h"
static int cbs_av1_read_uvlc(CodedBitstreamContext *ctx, GetBitContext *gbc,
diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
index 333d70ba4e..ae7f88a8a3 100644
--- a/libavcodec/cbs_vp9.c
+++ b/libavcodec/cbs_vp9.c
@@ -21,7 +21,6 @@
#include "cbs.h"
#include "cbs_internal.h"
#include "cbs_vp9.h"
-#include "internal.h"
static int cbs_vp9_read_s(CodedBitstreamContext *ctx, GetBitContext *gbc,
diff --git a/libavcodec/cljrenc.c b/libavcodec/cljrenc.c
index aa53a110d0..21e033753b 100644
--- a/libavcodec/cljrenc.c
+++ b/libavcodec/cljrenc.c
@@ -24,12 +24,10 @@
* Cirrus Logic AccuPak encoder.
*/
-#include "libavutil/common.h"
#include "libavutil/opt.h"
#include "avcodec.h"
#inclu
[FFmpeg-devel] [PATCH 3/7] avcodec/cbs: Make ff_cbs_insert_unit_data() always append the new unit
All split functions (the only users of this function) only
append units.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs.c | 22 +++---
libavcodec/cbs.h | 5 ++---
libavcodec/cbs_av1.c | 2 +-
libavcodec/cbs_h2645.c | 2 +-
libavcodec/cbs_jpeg.c | 2 +-
libavcodec/cbs_mpeg2.c | 2 +-
libavcodec/cbs_vp9.c | 4 ++--
7 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
index d7aa67c3af..e829caa0a0 100644
--- a/libavcodec/cbs.c
+++ b/libavcodec/cbs.c
@@ -789,18 +789,16 @@ int ff_cbs_insert_unit_content(CodedBitstreamFragment
*frag,
return 0;
}
-int ff_cbs_insert_unit_data(CodedBitstreamFragment *frag,
-int position,
-CodedBitstreamUnitType type,
-uint8_t *data, size_t data_size,
-AVBufferRef *data_buf)
+static int cbs_insert_unit_data(CodedBitstreamFragment *frag,
+CodedBitstreamUnitType type,
+uint8_t *data, size_t data_size,
+AVBufferRef *data_buf,
+int position)
{
CodedBitstreamUnit *unit;
AVBufferRef *data_ref;
int err;
-if (position == -1)
-position = frag->nb_units;
av_assert0(position >= 0 && position <= frag->nb_units);
if (data_buf)
@@ -828,6 +826,16 @@ int ff_cbs_insert_unit_data(CodedBitstreamFragment *frag,
return 0;
}
+int ff_cbs_append_unit_data(CodedBitstreamFragment *frag,
+CodedBitstreamUnitType type,
+uint8_t *data, size_t data_size,
+AVBufferRef *data_buf)
+{
+return cbs_insert_unit_data(frag, type,
+data, data_size, data_buf,
+frag->nb_units);
+}
+
void ff_cbs_delete_unit(CodedBitstreamFragment *frag,
int position)
{
diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
index 87ea14cd07..5583063b5e 100644
--- a/libavcodec/cbs.h
+++ b/libavcodec/cbs.h
@@ -393,14 +393,13 @@ int ff_cbs_insert_unit_content(CodedBitstreamFragment
*frag,
AVBufferRef *content_buf);
/**
- * Insert a new unit into a fragment with the given data bitstream.
+ * Add a new unit to a fragment with the given data bitstream.
*
* If data_buf is not supplied then data must have been allocated with
* av_malloc() and will on success become owned by the unit after this
* call or freed on error.
*/
-int ff_cbs_insert_unit_data(CodedBitstreamFragment *frag,
-int position,
+int ff_cbs_append_unit_data(CodedBitstreamFragment *frag,
CodedBitstreamUnitType type,
uint8_t *data, size_t data_size,
AVBufferRef *data_buf);
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 302e1f38f5..04314443de 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -828,7 +828,7 @@ static int cbs_av1_split_fragment(CodedBitstreamContext
*ctx,
goto fail;
}
-err = ff_cbs_insert_unit_data(frag, -1, header.obu_type,
+err = ff_cbs_append_unit_data(frag, header.obu_type,
data, obu_length, frag->data_ref);
if (err < 0)
goto fail;
diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
index 3396c059c9..10b3bcc70b 100644
--- a/libavcodec/cbs_h2645.c
+++ b/libavcodec/cbs_h2645.c
@@ -493,7 +493,7 @@ static int
cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx,
ref = (nal->data == nal->raw_data) ? frag->data_ref
: packet->rbsp.rbsp_buffer_ref;
-err = ff_cbs_insert_unit_data(frag, -1, nal->type,
+err = ff_cbs_append_unit_data(frag, nal->type,
(uint8_t*)nal->data, size, ref);
if (err < 0)
return err;
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index ae263ba038..da7ee808cf 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -226,7 +226,7 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext
*ctx,
data_ref = frag->data_ref;
}
-err = ff_cbs_insert_unit_data(frag, -1, marker,
+err = ff_cbs_append_unit_data(frag, marker,
data, data_size, data_ref);
if (err < 0)
return err;
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 4395bbf047..2bf38c6001 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -186,7 +186,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
final = 1;
}
-err = ff_cbs_insert_unit_data(frag, -1, unit_type, (uint8_t*)start,
+err = ff_cbs_append_unit_data(frag, uni
[FFmpeg-devel] [PATCH 4/7] avcodec/cbs_mpeg2: Simplify splitting fragment
avpriv_find_start_code() supports non-contiguous buffers
by maintaining a state that allows to find start codes
that span across multiple buffers; a consequence thereof
is that avpriv_find_start_code() is given a zero-sized
buffer, it does not modify this state, so that it appears
as if a start code was found if the state contained a start code.
This can e.g. happen with Sequence End units in MPEG-2 and
to counter this, cbs_mpeg2_split_fragment() reset the state
when it has already encountered the end of the fragment
in order to add the last unit (if it is only of the form 00 00 01 xy)
only once; it also used a flag to set whether this is the final unit.
Yet this can be improved by simply resetting state unconditionally
(thereby avoiding a branch); the flag can be removed by just checking
whether we have a valid start code (of the next unit to add)
at the end.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 2bf38c6001..90d667ddc8 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -149,7 +149,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
uint32_t start_code = -1;
size_t unit_size;
int err;
-int final = 0;
start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
&start_code);
@@ -161,14 +160,11 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
do {
unit_type = start_code & 0xff;
-if (start == frag->data + frag->data_size) {
-// The last four bytes form a start code which constitutes
-// a unit of its own. In this situation avpriv_find_start_code
-// won't modify start_code at all so modify start_code so that
-// the next unit will be treated as the last unit.
-start_code = 0;
-}
-
+// Reset start_code to ensure that avpriv_find_start_code()
+// really reads a new start code and does not reuse the old
+// start code in any way (as e.g. happens when there is a
+// Sequence End unit at the very end of a packet).
+start_code = UINT32_MAX;
end = avpriv_find_start_code(start--, frag->data + frag->data_size,
&start_code);
@@ -183,7 +179,6 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
} else {
// We didn't find a start code, so this is the final unit.
unit_size = end - start;
- final = 1;
}
err = ff_cbs_append_unit_data(frag, unit_type, (uint8_t*)start,
@@ -192,7 +187,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
return err;
start = end;
-} while (!final);
+} while ((start_code >> 8) == 0x01);
return 0;
}
--
2.32.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/7] avcodec/cbs_jpeg: Remove redundant counter
Use -1 as the position in ff_cbs_insert_unit_data()
which implicitly reuses frag->nb_units as the counter.
Also switch to a do-while-loop, as it is more natural
than a for-loop now that the counter is gone.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_jpeg.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index 7d3e10fcc8..ae263ba038 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -110,7 +110,7 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext
*ctx,
AVBufferRef *data_ref;
uint8_t *data;
size_t data_size;
-int unit, start, end, marker, next_start, next_marker;
+int start, end, marker, next_start, next_marker;
int err, i, j, length;
if (frag->data_size < 4) {
@@ -144,7 +144,7 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext
*ctx,
marker = frag->data[i];
start = i + 1;
-for (unit = 0;; unit++) {
+do {
if (marker == JPEG_MARKER_EOI) {
break;
} else if (marker == JPEG_MARKER_SOS) {
@@ -226,16 +226,14 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext
*ctx,
data_ref = frag->data_ref;
}
-err = ff_cbs_insert_unit_data(frag, unit, marker,
+err = ff_cbs_insert_unit_data(frag, -1, marker,
data, data_size, data_ref);
if (err < 0)
return err;
-if (next_marker == -1)
-break;
marker = next_marker;
start = next_start;
-}
+} while (next_marker != -1);
return 0;
}
--
2.32.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 1/7] avcodec/cbs_mpeg2: Remove redundant counter
Use -1 as the position in ff_cbs_insert_unit_data()
which implicitly reuses frag->nb_units as the counter.
Also switch to a do-while-loop, as it is more natural
than a for-loop now that the counter is gone.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cbs_mpeg2.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
index 26400f279f..4395bbf047 100644
--- a/libavcodec/cbs_mpeg2.c
+++ b/libavcodec/cbs_mpeg2.c
@@ -148,7 +148,8 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
CodedBitstreamUnitType unit_type;
uint32_t start_code = -1;
size_t unit_size;
-int err, i, final = 0;
+int err;
+int final = 0;
start = avpriv_find_start_code(frag->data, frag->data + frag->data_size,
&start_code);
@@ -157,7 +158,7 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
return AVERROR_INVALIDDATA;
}
-for (i = 0;; i++) {
+do {
unit_type = start_code & 0xff;
if (start == frag->data + frag->data_size) {
@@ -185,16 +186,13 @@ static int cbs_mpeg2_split_fragment(CodedBitstreamContext
*ctx,
final = 1;
}
-err = ff_cbs_insert_unit_data(frag, i, unit_type, (uint8_t*)start,
+err = ff_cbs_insert_unit_data(frag, -1, unit_type, (uint8_t*)start,
unit_size, frag->data_ref);
if (err < 0)
return err;
-if (final)
-break;
-
start = end;
-}
+} while (!final);
return 0;
}
--
2.32.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 1/2] avformat/matroskadec: Fix infinite loop with bz decompression
On Fri, Feb 04, 2022 at 04:29:18AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: Infinite loop
> > Fixes:
> > 43932/clusterfuzz-testcase-minimized-ffmpeg_dem_MATROSKA_fuzzer-6175167573786624
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer
> > ---
> > libavformat/matroskadec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> > index d165f6ab90..5a9acfb247 100644
> > --- a/libavformat/matroskadec.c
> > +++ b/libavformat/matroskadec.c
> > @@ -1742,7 +1742,7 @@ static int matroska_decode_buffer(uint8_t **buf, int
> > *buf_size,
> > case MATROSKA_TRACK_ENCODING_COMP_BZLIB:
> > {
> > bz_stream bzstream = { 0 };
> > -if (BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK)
> > +if (!pkt_size || BZ2_bzDecompressInit(&bzstream, 0, 0) != BZ_OK)
> > return -1;
> > bzstream.next_in = data;
> > bzstream.avail_in = isize;
>
> I see nothing in the zlib-API manual that would preclude this from
> happening with zlib, too, so it should be checked there, too.
> LGTM apart from that.
It didnt happen with the same case when i just routed it to the zlib
instead of bz case. But i didnt look what happened exactly
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.
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 1/1] avformat: Add IPFS protocol support.
On Fri, Feb 4, 2022 at 11:29 AM Tomas Härdin wrote:
> ons 2022-02-02 klockan 14:56 +0100 skrev Mark Gaiser:
> > On Wed, Feb 2, 2022 at 2:21 PM Tomas Härdin
> > wrote:
> >
> > > tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser:
> > >
> > > >
> > > > +typedef struct Context {
> > > > +AVClass *class;
> > > > +URLContext *inner;
> > > > +char *gateway;
> > >
> > > Is there not a maximum length that an HTTP URL can be? At least
> > > without
> > > query parameters. That way you avoid dynamic allocations. You'd
> > > have to
> > > separate the AVOption from such a buffer in that case, but I think
> > > you
> > > have to anyway.
> > >
> >
> > Could you provide more information on that? Or an example of what you
> > mean
> > exactly?
> > As far as i know there is no hard limit though it's very much advised
> > to
> > not go above 2048 characters.
>
> You could at least separate the variable that is set by the AVOption
> stuff from the buffer that's produced by the URL parsing stuff.
>
> Since you call ffurl_open_whitelist() immediately you could just use
> alloca() to allocate memory on the stack. That way you don't have to
> worry about deallocation at all *and* you can have it be arbitrarily
> large.
>
> If URLs have a spec'd maximum length then a buffer of constant size on
> the stack would work.
>
> >
> > >
> > > > +if (!ipfs_full_data_folder) {
> > > > +av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> > > > +
> > > > +// Try via the home folder.
> > > > +home_folder = getenv("HOME");
> > > > +ipfs_full_data_folder = av_asprintf("%s/.ipfs/",
> > > > home_folder);
> > >
> > > Memory leak. This applies to most if not all av_asprintf() calls.
> > >
> >
> > Is there an advised way to neatly clean that up?
> > Sure, I can add a bunch of av_free calls to clean it up. But there
> > are
> > places where it's not as straightforward like where the av_asprintf
> > was
> > done in an if statement. How do I maintain the knowledge that
> > av_asprintf
> > was used to call av_free later?
>
> goto as the others pointed out
>
>
> >
> > >
> > > > +static int translate_ipfs_to_http(URLContext *h, const char
> > > > *uri,
> > > > int flags, AVDictionary **options)
> > > > +{
> > > > +const char *ipfs_cid;
> > > > +const char *protocol_path_suffix = "ipfs/";
> > > > +char *fulluri;
> > > > +int ret;
> > > > +Context *c = h->priv_data;
> > > > +int is_ipfs = (av_strstart(uri, "ipfs://", &ipfs_cid) ||
> > > > av_strstart(uri, "ipfs:", &ipfs_cid));
> > > > +int is_ipns = (av_strstart(uri, "ipns://", &ipfs_cid) ||
> > > > av_strstart(uri, "ipns:", &ipfs_cid));
> > >
> > > https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the
> > > canonical form. No mentioned is made of any ipfs:{CID} form.
> > > Incorrect
> > > URLs should be rejected, not silently patched.
> > >
> >
> > I'd like to make a decision here. This current logic (ipfs:// and
> > ipfs:,
> > same for ipns) is inspired by other protocols that ffmpeg supported.
> > I
> > simply copied how they work to be consistent.
> > Do i:
> > 1. keep it as is and be consistent with the rest?
> > 2. only allow ipfs:// and ipns://?
> >
>
> Which protocols? This laxness in lavf is worrisome. It breeds laxness
> in other projects.
>
Hmm, I might have been looking at the wrong example and calling that
"others do it too".
Crypto does it with "crypto:" and "crypto+"
From a quick glance that's about it.
I'll just remove "ipfs:". I only wanted to stay consistent with what I knew
(from crypto) but that is apparently inconsistent.
>
> /Tomas
>
> ___
> 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".
Re: [FFmpeg-devel] [PATCH v4 1/1] avformat: Add IPFS protocol support.
On Fri, Feb 4, 2022 at 12:10 PM Tomas Härdin wrote:
> tor 2022-02-03 klockan 18:29 +0100 skrev Mark Gaiser:
> >
> > +typedef struct IPFSGatewayContext {
> > +AVClass *class;
> > +URLContext *inner;
> > +char *gateway;
>
> Consider two separate variables. One for AVOption and one for the
> dynamically allocated string. Or put the latter on the stack.
>
There always needs to be a gateway so why is reusing that variable an issue?
I'm fine splitting it up but I'd like to understand the benefit of it as
currently I don't see that benefit.
> > +} IPFSGatewayContext;
> > +
> > +// A best-effort way to find the IPFS gateway.
> > +// Only the most appropiate gateway is set. It's not actually
> > requested
> > +// (http call) to prevent a potential slowdown in startup. A
> > potential timeout
> > +// is handled by the HTTP protocol.
> > +//
> > +// Return codes can be:
> > +// 1 : A potential gateway is found and set in c->gateway
> > +// -1: The IPFS data folder could not be found
> > +// -2: The gateway file could not be found
> > +// -3: The gateway file is found but empty
> > +// -4: $HOME is empty
> > +// -9: Unhandled error
>
> What Michael meant with better return codes is using AVERROR_* :)
>
Hey, first attempt at an ffmpeg contribution here. I didn't know that at
all. ;)
But yeah, will do!
> > +static int populate_ipfs_gateway(URLContext *h)
> > +{
> > +IPFSGatewayContext *c = h->priv_data;
> > +char *ipfs_full_data_folder = NULL;
> > +char *ipfs_gateway_file = NULL;
>
> These can be char[PATH_MAX]
>
Oke, will do.
C code question though.
How do I use av_asprintf on stack arrays like that?
I kinda like to prevent using something different just to later figure out
that there was an av_ function that would be far better :)
> > +struct stat st;
> > +int stat_ret = 0;
> > +int ret = -9;
> > +FILE *gateway_file = NULL;
> > +char gateway_file_data[1000];
>
> A maximum URL length of 999?
>
There doesn't seem to be a hard limit on url's..
It even seems to be browser dependent. Chrome apparently allows it up to
2MB, firefox up to 64k.
I'm just going to use PATH_MAX here too which seems plenty with 4096 bytes.
IPFS url's can be rather lengthy though so I do like to keep enough room
for it.
> > +
> > +// First, test if there already is a path in c->gateway. If it
> > is then it
> > +// was provided as cli arument and should be used. It takes
> > precdence.
> > +if (c->gateway != NULL) {
> > +ret = 1;
> > +goto err;
> > +}
> > +
> > +// Test $IPFS_GATEWAY.
> > +if (getenv("IPFS_GATEWAY") != NULL) {
> > +av_free(c->gateway);
>
> Useless since c->gateway is NULL
>
> > +
> > +// Stat the folder.
> > +// It should exist in a default IPFS setup when run as local
> > user.
> > +#ifndef _WIN32
> > +stat_ret = stat(ipfs_full_data_folder, &st);
> > +#else
> > +stat_ret = win32_stat(ipfs_full_data_folder, &st);
> > +#endif
>
> Again, there is no reason to stat this. Just try opening the gateway
> file directly.
>
This is a folder, not a file.
The other stat that was here too was a file, I replaced that with an fopen.
It smells sketchy to me to (ab)use fopen to check if a folder exists.
There's stat for that.
>
> > +
> > +// Read a single line (fgets stops at new line mark).
> > +fgets(gateway_file_data, sizeof(gateway_file_data) - 1,
> > gateway_file);
>
> This can result in gateway_file_data not being NUL terminated
> > +
> > +// Replace first occurence of end of line to \0
> > +gateway_file_data[strcspn(gateway_file_data, "\r\n")] = 0;
>
> What if the file uses \n or no newlines at all?
>
Right.
So I guess the fix here is:
1. Initialize gateway_file_data so all bytes are zero
2. read a line
3. set the last byte of gateway_file_data to 0
Now any text in the string will be the gateway.
Is that a proper fix?
> > +err:
> > +if (gateway_file)
> > +fclose(gateway_file);
> > +
> > +av_free(ipfs_full_data_folder);
> > +av_free(ipfs_gateway_file);
>
> This is not cleaning up dynamic allocations of c->gateway
>
So I should do that in ipfs_close, right?
>
> > +// -3: The gateway url part (without the protocol) is too short. We
> > expect 3
> > +// characters minimal. So http://aaa would be the bare minimal.
>
> http://1 is valid I think. It means http://0.0.0.1
Right, 1 character it is.
I thought I'd go for a common sense approach as in "it can't possibly be
smaller than..."
But I suppose just forcing any character to be there is fine too.
>
>
> > +// Test if the gateway starts with either http:// or https://
> > +// The remainder is stored in url_without_protocol
> > +if (av_stristart(uri, "http://";, &url_without_protocol) == 0
> > +&& av_stristart(uri, "https://";, &url_without_protocol) ==
> > 0) {
> > +av_log(h, AV_LOG_ERROR, "The gateway URL didn't start with
> > http:// or https:// and is therefore invalid.\n");
Re: [FFmpeg-devel] [PATCH v4 1/1] avformat: Add IPFS protocol support.
tor 2022-02-03 klockan 18:29 +0100 skrev Mark Gaiser:
>
> +typedef struct IPFSGatewayContext {
> + AVClass *class;
> + URLContext *inner;
> + char *gateway;
Consider two separate variables. One for AVOption and one for the
dynamically allocated string. Or put the latter on the stack.
> +} IPFSGatewayContext;
> +
> +// A best-effort way to find the IPFS gateway.
> +// Only the most appropiate gateway is set. It's not actually
> requested
> +// (http call) to prevent a potential slowdown in startup. A
> potential timeout
> +// is handled by the HTTP protocol.
> +//
> +// Return codes can be:
> +// 1 : A potential gateway is found and set in c->gateway
> +// -1: The IPFS data folder could not be found
> +// -2: The gateway file could not be found
> +// -3: The gateway file is found but empty
> +// -4: $HOME is empty
> +// -9: Unhandled error
What Michael meant with better return codes is using AVERROR_* :)
> +static int populate_ipfs_gateway(URLContext *h)
> +{
> + IPFSGatewayContext *c = h->priv_data;
> + char *ipfs_full_data_folder = NULL;
> + char *ipfs_gateway_file = NULL;
These can be char[PATH_MAX]
> + struct stat st;
> + int stat_ret = 0;
> + int ret = -9;
> + FILE *gateway_file = NULL;
> + char gateway_file_data[1000];
A maximum URL length of 999?
> +
> + // First, test if there already is a path in c->gateway. If it
> is then it
> + // was provided as cli arument and should be used. It takes
> precdence.
> + if (c->gateway != NULL) {
> + ret = 1;
> + goto err;
> + }
> +
> + // Test $IPFS_GATEWAY.
> + if (getenv("IPFS_GATEWAY") != NULL) {
> + av_free(c->gateway);
Useless since c->gateway is NULL
> +
> + // Stat the folder.
> + // It should exist in a default IPFS setup when run as local
> user.
> +#ifndef _WIN32
> + stat_ret = stat(ipfs_full_data_folder, &st);
> +#else
> + stat_ret = win32_stat(ipfs_full_data_folder, &st);
> +#endif
Again, there is no reason to stat this. Just try opening the gateway
file directly.
> +
> + // Read a single line (fgets stops at new line mark).
> + fgets(gateway_file_data, sizeof(gateway_file_data) - 1,
> gateway_file);
This can result in gateway_file_data not being NUL terminated
> +
> + // Replace first occurence of end of line to \0
> + gateway_file_data[strcspn(gateway_file_data, "\r\n")] = 0;
What if the file uses \n or no newlines at all?
> +err:
> + if (gateway_file)
> + fclose(gateway_file);
> +
> + av_free(ipfs_full_data_folder);
> + av_free(ipfs_gateway_file);
This is not cleaning up dynamic allocations of c->gateway
> +// -3: The gateway url part (without the protocol) is too short. We
> expect 3
> +// characters minimal. So http://aaa would be the bare minimal.
http://1 is valid I think. It means http://0.0.0.1
> + // Test if the gateway starts with either http:// or https://
> + // The remainder is stored in url_without_protocol
> + if (av_stristart(uri, "http://";, &url_without_protocol) == 0
> + && av_stristart(uri, "https://";, &url_without_protocol) ==
> 0) {
> + av_log(h, AV_LOG_ERROR, "The gateway URL didn't start with
> http:// or https:// and is therefore invalid.\n");
> + ret = -2;
> + goto err;
> + }
I guess restricting this to HTTP schemes is OK. Or are there non-HTTP
gateways for this?
> + if (last_gateway_char != '/') {
> + c->gateway = av_asprintf("%s/", c->gateway);
Yet another leak
> // Sanitize the gateway to a format we expect.
> +if (sanitize_ipfs_gateway(h) < 1)
> +goto err;
This will return unset ret, thus leaking data from the stack
> +static int ipfs_close(URLContext *h)
> +{
> + IPFSGatewayContext *c = h->priv_data;
Here is where you'd put any deallocations
The quality of this patch is making me re-affirm what I've already said
viz parsing. bash+sed is superior.
/Tomas
___
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 2/2] libavformat/mov: fix udta reading in trak box
On Fri, Feb 4, 2022 at 5:24 AM Wang Chuan wrote:
>
> Ping?
> On Jan 28, 2022, 11:24 AM +0800, Wang Chuan , wrote:
> > if we are reading udta in trak box, the data should go to metadata
> > of current stream.
> >
> > Signed-off-by: Wang Chuan
> > ---
> > libavformat/mov.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 1437d160f8..cb983defb3 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -522,7 +522,10 @@ retry:
> > str[str_size] = 0;
> > }
> > c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
> > - av_dict_set(&c->fc->metadata, key, str, 0);
> > + if (c->trak_index != -1)
> > + av_dict_set(&c->fc->streams[c->trak_index]->metadata, key,
> > str, 0);
> > + else
> > + av_dict_set(&c->fc->metadata, key, str, 0);
> > if (*language && strcmp(language, "und")) {
> > snprintf(key2, sizeof(key2), "%s-%s", key, language);
> > av_dict_set(&c->fc->metadata, key2, str, 0);
> > --
> > 2.29.2
I recall having some patches on my github regarding something related,
will attempt to check this during the week-end.
Jan
___
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 1/4] avformat/demux: don't propagate unsupported skip samples packet side data values
On 1/31/2022 10:56 PM, James Almer wrote:
Should fix ticket #9622
Signed-off-by: James Almer
---
I'm not sure if this is ok or not. The AV_PKT_DATA_SKIP_SAMPLES doxy states
the skip samples value is a little endian uint32 value, so even if the mov
demuxer wrote a truncated int64_t value in sti->skip_samples (which, being an
int, can be negative), it would still be "valid" when written into the packet
as side data.
Chromium's fix is
https://chromium-review.googlesource.com/c/chromium/src/+/3424556
where you can see they have been reading the skip samples value from
AV_PKT_DATA_SKIP_SAMPLES as an int.
libavformat/demux.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavformat/demux.c b/libavformat/demux.c
index f895f0ba85..09d539af68 100644
--- a/libavformat/demux.c
+++ b/libavformat/demux.c
@@ -1354,6 +1354,7 @@ static int read_frame_internal(AVFormatContext *s,
AVPacket *pkt)
}
if (sti->start_skip_samples && (pkt->pts == 0 || pkt->pts ==
RELATIVE_TS_BASE))
sti->skip_samples = sti->start_skip_samples;
+sti->skip_samples = FFMAX(0, sti->skip_samples);
if (sti->skip_samples || discard_padding) {
uint8_t *p = av_packet_new_side_data(pkt,
AV_PKT_DATA_SKIP_SAMPLES, 10);
if (p) {
Will apply patchset.
___
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 1/1] avformat: Add IPFS protocol support.
ons 2022-02-02 klockan 14:56 +0100 skrev Mark Gaiser:
> On Wed, Feb 2, 2022 at 2:21 PM Tomas Härdin
> wrote:
>
> > tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser:
> >
> > >
> > > +typedef struct Context {
> > > + AVClass *class;
> > > + URLContext *inner;
> > > + char *gateway;
> >
> > Is there not a maximum length that an HTTP URL can be? At least
> > without
> > query parameters. That way you avoid dynamic allocations. You'd
> > have to
> > separate the AVOption from such a buffer in that case, but I think
> > you
> > have to anyway.
> >
>
> Could you provide more information on that? Or an example of what you
> mean
> exactly?
> As far as i know there is no hard limit though it's very much advised
> to
> not go above 2048 characters.
You could at least separate the variable that is set by the AVOption
stuff from the buffer that's produced by the URL parsing stuff.
Since you call ffurl_open_whitelist() immediately you could just use
alloca() to allocate memory on the stack. That way you don't have to
worry about deallocation at all *and* you can have it be arbitrarily
large.
If URLs have a spec'd maximum length then a buffer of constant size on
the stack would work.
>
> >
> > > + if (!ipfs_full_data_folder) {
> > > + av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> > > +
> > > + // Try via the home folder.
> > > + home_folder = getenv("HOME");
> > > + ipfs_full_data_folder = av_asprintf("%s/.ipfs/",
> > > home_folder);
> >
> > Memory leak. This applies to most if not all av_asprintf() calls.
> >
>
> Is there an advised way to neatly clean that up?
> Sure, I can add a bunch of av_free calls to clean it up. But there
> are
> places where it's not as straightforward like where the av_asprintf
> was
> done in an if statement. How do I maintain the knowledge that
> av_asprintf
> was used to call av_free later?
goto as the others pointed out
>
> >
> > > +static int translate_ipfs_to_http(URLContext *h, const char
> > > *uri,
> > > int flags, AVDictionary **options)
> > > +{
> > > + const char *ipfs_cid;
> > > + const char *protocol_path_suffix = "ipfs/";
> > > + char *fulluri;
> > > + int ret;
> > > + Context *c = h->priv_data;
> > > + int is_ipfs = (av_strstart(uri, "ipfs://", &ipfs_cid) ||
> > > av_strstart(uri, "ipfs:", &ipfs_cid));
> > > + int is_ipns = (av_strstart(uri, "ipns://", &ipfs_cid) ||
> > > av_strstart(uri, "ipns:", &ipfs_cid));
> >
> > https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the
> > canonical form. No mentioned is made of any ipfs:{CID} form.
> > Incorrect
> > URLs should be rejected, not silently patched.
> >
>
> I'd like to make a decision here. This current logic (ipfs:// and
> ipfs:,
> same for ipns) is inspired by other protocols that ffmpeg supported.
> I
> simply copied how they work to be consistent.
> Do i:
> 1. keep it as is and be consistent with the rest?
> 2. only allow ipfs:// and ipns://?
>
Which protocols? This laxness in lavf is worrisome. It breeds laxness
in other projects.
/Tomas
___
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 0/5] Add IPFS and IPNS protocol support
ons 2022-02-02 klockan 14:48 +0100 skrev Michael Niedermayer: > On Tue, Feb 01, 2022 at 05:43:24PM +0100, Tomas Härdin wrote: > > tis 2022-02-01 klockan 11:06 +0100 skrev Michael Niedermayer: > > > On Mon, Jan 31, 2022 at 09:22:52PM +0100, Tomas Härdin wrote: > > > [...] > > > > It strikes me that this borders on incorporating business logic > > > > within > > > > lavf. A user could achieve the same thing with a small shell > > > > script. > > > > For example adding an alias that inspects calls to ffmpeg and > > > > sed:s > > > > ipfs:// URLs accordingly > > > > > > That sounds like a security nightmare > > > > Parsing shit in C is a far bigger nightmare I can assure you. The > > command line can leverage sed and the regex in the URL RFC. > > the problem is if you parse it on the command line and change it > before passing it to ffmpeg. You really need to have ffmpeg and > the command line parse it 100% exactly the same. The word you're looking for is langsec. It is why I harp on about being very strict with what we accept. /Tomas ___ 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] zscale video filter performance optimization 4x
Optimizations:
by ffmpeg threading support implementation via frame slicing and moving
zimg_filter_graph_build to the filter initialization phase from each frame
processig
the performance increase vs original version
in video downscale and color conversion up to 4x is seen
on 64 cores Intel Xeon, 3x on i7-6700K (4 cores with HT)
Signed-off-by: Victoria Zhislina
---
libavfilter/vf_zscale.c | 779 ++--
1 file changed, 433 insertions(+), 346 deletions(-)
diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 1288c5efc1..1a2de1fe21 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -1,6 +1,7 @@
/*
* Copyright (c) 2015 Paul B Mahol
- *
+ * * 2022 Victoria Zhislina, Intel Corporation - performance optimization
+
* This file is part of FFmpeg.
*
* FFmpeg is free software; you can redistribute it and/or
@@ -44,6 +45,8 @@
#include "libavutil/imgutils.h"
#define ZIMG_ALIGNMENT 32
+#define MIN_TILESIZE 64
+#define MAX_THREADS 64
static const char *const var_names[] = {
"in_w", "iw",
@@ -113,13 +116,14 @@ typedef struct ZScaleContext {
int force_original_aspect_ratio;
-void *tmp;
-size_t tmp_size;
+void *tmp[MAX_THREADS]; //separate for each thread;
+int nb_threads;
+int slice_h;
zimg_image_format src_format, dst_format;
zimg_image_format alpha_src_format, alpha_dst_format;
zimg_graph_builder_params alpha_params, params;
-zimg_filter_graph *alpha_graph, *graph;
+zimg_filter_graph *alpha_graph[MAX_THREADS], *graph[MAX_THREADS];
enum AVColorSpace in_colorspace, out_colorspace;
enum AVColorTransferCharacteristic in_trc, out_trc;
@@ -128,10 +132,167 @@ typedef struct ZScaleContext {
enum AVChromaLocation in_chromal, out_chromal;
} ZScaleContext;
+
+typedef struct ThreadData {
+const AVPixFmtDescriptor *desc, *odesc;
+AVFrame *in, *out;
+} ThreadData;
+
+static int convert_chroma_location(enum AVChromaLocation chroma_location)
+{
+switch (chroma_location) {
+case AVCHROMA_LOC_UNSPECIFIED:
+case AVCHROMA_LOC_LEFT:
+return ZIMG_CHROMA_LEFT;
+case AVCHROMA_LOC_CENTER:
+return ZIMG_CHROMA_CENTER;
+case AVCHROMA_LOC_TOPLEFT:
+return ZIMG_CHROMA_TOP_LEFT;
+case AVCHROMA_LOC_TOP:
+return ZIMG_CHROMA_TOP;
+case AVCHROMA_LOC_BOTTOMLEFT:
+return ZIMG_CHROMA_BOTTOM_LEFT;
+case AVCHROMA_LOC_BOTTOM:
+return ZIMG_CHROMA_BOTTOM;
+}
+return ZIMG_CHROMA_LEFT;
+}
+
+static int convert_matrix(enum AVColorSpace colorspace)
+{
+switch (colorspace) {
+case AVCOL_SPC_RGB:
+return ZIMG_MATRIX_RGB;
+case AVCOL_SPC_BT709:
+return ZIMG_MATRIX_709;
+case AVCOL_SPC_UNSPECIFIED:
+return ZIMG_MATRIX_UNSPECIFIED;
+case AVCOL_SPC_FCC:
+return ZIMG_MATRIX_FCC;
+case AVCOL_SPC_BT470BG:
+return ZIMG_MATRIX_470BG;
+case AVCOL_SPC_SMPTE170M:
+return ZIMG_MATRIX_170M;
+case AVCOL_SPC_SMPTE240M:
+return ZIMG_MATRIX_240M;
+case AVCOL_SPC_YCGCO:
+return ZIMG_MATRIX_YCGCO;
+case AVCOL_SPC_BT2020_NCL:
+return ZIMG_MATRIX_2020_NCL;
+case AVCOL_SPC_BT2020_CL:
+return ZIMG_MATRIX_2020_CL;
+case AVCOL_SPC_CHROMA_DERIVED_NCL:
+return ZIMG_MATRIX_CHROMATICITY_DERIVED_NCL;
+case AVCOL_SPC_CHROMA_DERIVED_CL:
+return ZIMG_MATRIX_CHROMATICITY_DERIVED_CL;
+case AVCOL_SPC_ICTCP:
+return ZIMG_MATRIX_ICTCP;
+}
+return ZIMG_MATRIX_UNSPECIFIED;
+}
+
+static int convert_trc(enum AVColorTransferCharacteristic color_trc)
+{
+switch (color_trc) {
+case AVCOL_TRC_UNSPECIFIED:
+return ZIMG_TRANSFER_UNSPECIFIED;
+case AVCOL_TRC_BT709:
+return ZIMG_TRANSFER_709;
+case AVCOL_TRC_GAMMA22:
+return ZIMG_TRANSFER_470_M;
+case AVCOL_TRC_GAMMA28:
+return ZIMG_TRANSFER_470_BG;
+case AVCOL_TRC_SMPTE170M:
+return ZIMG_TRANSFER_601;
+case AVCOL_TRC_SMPTE240M:
+return ZIMG_TRANSFER_240M;
+case AVCOL_TRC_LINEAR:
+return ZIMG_TRANSFER_LINEAR;
+case AVCOL_TRC_LOG:
+return ZIMG_TRANSFER_LOG_100;
+case AVCOL_TRC_LOG_SQRT:
+return ZIMG_TRANSFER_LOG_316;
+case AVCOL_TRC_IEC61966_2_4:
+return ZIMG_TRANSFER_IEC_61966_2_4;
+case AVCOL_TRC_BT2020_10:
+return ZIMG_TRANSFER_2020_10;
+case AVCOL_TRC_BT2020_12:
+return ZIMG_TRANSFER_2020_12;
+case AVCOL_TRC_SMPTE2084:
+return ZIMG_TRANSFER_ST2084;
+case AVCOL_TRC_ARIB_STD_B67:
+return ZIMG_TRANSFER_ARIB_B67;
+case AVCOL_TRC_IEC61966_2_1:
+return ZIMG_TRANSFER_IEC_61966_2_1;
+}
+return ZIMG_TRANSFER_UNSPECIFIED;
+}
+
+static int convert_primaries(enum AVColorPrimaries color_primaries)
+{
+switch (color_primaries) {
+case AVCOL_PRI_UNSPECIFIED:
+return ZIMG_PRIMARIES_UNSPECIFIED;
+case AVCOL_PRI_BT709:
+return ZIMG_PRIMARIES
