Re: [FFmpeg-devel] [PATCH 1/7] avcodec/proresdec: Include required headers directly
Andreas Rheinhardt:
> Do not rely on an indirect inclusion of avcodec.h in proresdsp.h.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/proresdec.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/proresdec.h b/libavcodec/proresdec.h
> index 1e48752e6f..230fca41f2 100644
> --- a/libavcodec/proresdec.h
> +++ b/libavcodec/proresdec.h
> @@ -22,10 +22,15 @@
> #ifndef AVCODEC_PRORESDEC_H
> #define AVCODEC_PRORESDEC_H
>
> +#include
> +
> #include "get_bits.h"
> #include "blockdsp.h"
> #include "proresdsp.h"
>
> +#include "libavutil/frame.h"
> +#include "libavutil/pixfmt.h"
> +
> typedef struct {
> const uint8_t *data;
> unsigned mb_x;
Will apply this patchset tomorrow unless there are objections.
- 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 01/21] avformat/avio: Don't use incompatible function pointer type for call
On Sat, 9 Sep 2023, Tomas Härdin wrote: fre 2023-09-08 klockan 22:38 +0200 skrev Marton Balint: On Thu, 7 Sep 2023, Andreas Rheinhardt wrote: > It is undefined behaviour even in cases where it works > (it works because it is only a const uint8_t* vs. uint8_t* > difference). > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/avio.c | 25 - > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/libavformat/avio.c b/libavformat/avio.c > index ab1c19a58d..d53da5cb0c 100644 > --- a/libavformat/avio.c > +++ b/libavformat/avio.c > @@ -354,10 +354,15 @@ fail: > } > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t > *buf, > + const uint8_t *cbuf, > int size, int size_min, > - int > (*transfer_func)(URLContext *h, > - > uint8_t *buf, > - int > size)) > + int > (*read_func)(URLContext *h, > + uint8_t > *buf, > + int > size), > + int > (*write_func)(URLContext *h, > + const > uint8_t *buf, > + int > size), > + int read) These extra parameters are very ugly, can't we think of another way to properly support this? One idea is putting retry_transfer_wrapper in a template file and include it twice with proper defines-s for the read and write flavours. Seems like a lot of work for a function that's internal to avio.c If future extensibility is not important here then function pointers should not be passed to retry_tranfer_wrapper because h->prot->url_read/write can be used directly. And usage of buf/cbuf is readundant with the read paramter, because by checking if buf or cbuf is null you can decide the operation (read of write). Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/21] avformat/avio: Don't use incompatible function pointer type for call
Marton Balint: > > > On Sat, 9 Sep 2023, Tomas Härdin wrote: > >> fre 2023-09-08 klockan 22:38 +0200 skrev Marton Balint: >>> >>> >>> On Thu, 7 Sep 2023, Andreas Rheinhardt wrote: >>> >>> > It is undefined behaviour even in cases where it works >>> > (it works because it is only a const uint8_t* vs. uint8_t* >>> > difference). >>> > > Signed-off-by: Andreas Rheinhardt >>> > --- >>> > libavformat/avio.c | 25 - >>> > 1 file changed, 16 insertions(+), 9 deletions(-) >>> > > diff --git a/libavformat/avio.c b/libavformat/avio.c >>> > index ab1c19a58d..d53da5cb0c 100644 >>> > --- a/libavformat/avio.c >>> > +++ b/libavformat/avio.c >>> > @@ -354,10 +354,15 @@ fail: >>> > } >>> > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t >>> > *buf, >>> > + const uint8_t *cbuf, >>> > int size, int size_min, >>> > - int >>> > (*transfer_func)(URLContext *h, >>> > - >>> > uint8_t *buf, >>> > - int >>> > size)) >>> > + int >>> > (*read_func)(URLContext *h, >>> > + uint8_t >>> > *buf, >>> > + int >>> > size), >>> > + int >>> > (*write_func)(URLContext *h, >>> > + const >>> > uint8_t *buf, >>> > + int >>> > size), >>> > + int read) >>> >>> These extra parameters are very ugly, can't we think of another way >>> to properly support this? >>> >>> One idea is putting retry_transfer_wrapper in a template file and >>> include it twice with proper defines-s for the read and write flavours. >> >> Seems like a lot of work for a function that's internal to avio.c > > If future extensibility is not important here then function pointers > should not be passed to retry_tranfer_wrapper because > h->prot->url_read/write can be used directly. And usage of buf/cbuf is > readundant with the read paramter, because by checking if buf or cbuf is > null you can decide the operation (read of write). > The compiler does not know whether buf given to ffurl_(read|write|read_complete) is NULL or not in the first place (it also does not know whether the url_read and url_write function pointers are NULL or not); therefore if one use e.g. cbuf != NULL as meaning read == 0, then the write function would actually check for whether cbuf is NULL which is worse than it is now. (My initial version (not sent to this list) checked for whether the read function was NULL in order to determine whether we are reading or writing; the assumption was that the compiler would optimize the check away when reading, because if the read function were NULL, then a NULL function pointer would be used for a call, which is undefined behaviour. But it didn't. Instead it added ffurl_read.cold and ffurl_read_complete.cold functions (which presumably abort or so) for this case.) - 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".
[FFmpeg-devel] Trac spam
new spammer in ticket 2104 ___ 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 01/21] avformat/avio: Don't use incompatible function pointer type for call
Tomas Härdin: > tor 2023-09-07 klockan 02:23 +0200 skrev Andreas Rheinhardt: >> It is undefined behaviour even in cases where it works >> (it works because it is only a const uint8_t* vs. uint8_t* >> difference). >> >> Signed-off-by: Andreas Rheinhardt >> --- >> libavformat/avio.c | 25 - >> 1 file changed, 16 insertions(+), 9 deletions(-) > > Looks OK. It's probably possible to get around the need for cbuf by > casting to/from uintptr_t, but using cbuf is more type safe > I just sent a new version to accomodate Marton's objections; are you ok with it? - 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".
[FFmpeg-devel] [PATCH v2] avformat/avio: Don't use incompatible function pointer type for call
It is undefined behaviour even in cases where it works
(it works because it is only a const uint8_t* vs. uint8_t* difference).
Instead use a macro to produce two functions with the required
types to be const-correct and type-safe.
Signed-off-by: Andreas Rheinhardt
---
libavformat/avio.c | 98 +++---
1 file changed, 50 insertions(+), 48 deletions(-)
diff --git a/libavformat/avio.c b/libavformat/avio.c
index ab1c19a58d..9783cb1881 100644
--- a/libavformat/avio.c
+++ b/libavformat/avio.c
@@ -353,63 +353,67 @@ fail:
return ret;
}
-static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf,
- int size, int size_min,
- int (*transfer_func)(URLContext *h,
- uint8_t *buf,
- int size))
-{
-int ret, len;
-int fast_retries = 5;
-int64_t wait_since = 0;
-
-len = 0;
-while (len < size_min) {
-if (ff_check_interrupt(&h->interrupt_callback))
-return AVERROR_EXIT;
-ret = transfer_func(h, buf + len, size - len);
-if (ret == AVERROR(EINTR))
-continue;
-if (h->flags & AVIO_FLAG_NONBLOCK)
-return ret;
-if (ret == AVERROR(EAGAIN)) {
-ret = 0;
-if (fast_retries) {
-fast_retries--;
-} else {
-if (h->rw_timeout) {
-if (!wait_since)
-wait_since = av_gettime_relative();
-else if (av_gettime_relative() > wait_since +
h->rw_timeout)
-return AVERROR(EIO);
-}
-av_usleep(1000);
-}
-} else if (ret == AVERROR_EOF)
-return (len > 0) ? len : AVERROR_EOF;
-else if (ret < 0)
-return ret;
-if (ret) {
-fast_retries = FFMAX(fast_retries, 2);
-wait_since = 0;
-}
-len += ret;
-}
-return len;
+#define RETRY_TRANSFER_WRAPPER(RW, CONST) \
+static inline int retry_ ## RW ## _wrapper(URLContext *h, CONST uint8_t *buf, \
+ int size, int size_min,\
+ int (*RW ## _func)(URLContext *h, \
+ CONST uint8_t
*buf, \
+ int size)) \
+{ \
+int ret, len; \
+int fast_retries = 5; \
+int64_t wait_since = 0; \
+ \
+len = 0; \
+while (len < size_min) { \
+if (ff_check_interrupt(&h->interrupt_callback)) \
+return AVERROR_EXIT; \
+ret = RW ## _func(h, buf + len, size - len); \
+if (ret == AVERROR(EINTR))\
+continue; \
+if (h->flags & AVIO_FLAG_NONBLOCK)\
+return ret; \
+if (ret == AVERROR(EAGAIN)) { \
+ret = 0; \
+if (fast_retries) { \
+fast_retries--; \
+} else { \
+if (h->rw_timeout) { \
+if (!wait_since) \
+wait_since = av_gettime_relative(); \
+else if (av_gettime_relative() > wait_since +
h->rw_timeout) \
+return AVERROR(EIO); \
+} \
+av_usleep(1000); \
+} \
+} else if (ret == AVERROR_EOF)\
+return (len > 0) ? len : AVERROR_EOF; \
+else if (ret <
Re: [FFmpeg-devel] [PATCH v2 01/22] fate/demux, lavf-container: Workaround for AV1-aspect ratio issue
Andreas Rheinhardt:
> This is a workaround for an issue introduced in commit
> 1652f2492f88434010053289d946dab6a57e4d58. It is not meant to be applied
> to the tree.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> tests/fate-run.sh | 2 +-
> tests/fate/demux.mak | 2 +-
> tests/fate/lavf-container.mak | 4 ++--
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> index 5a71ac001e..276f97489d 100755
> --- a/tests/fate-run.sh
> +++ b/tests/fate-run.sh
> @@ -383,7 +383,7 @@ lavf_container_fate()
> file=${outdir}/lavf.$t
> cleanfiles="$cleanfiles $file"
> input="${target_samples}/$1"
> -do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input"
> "$ENC_OPTS -metadata title=lavftest" -vcodec copy -acodec copy
> +do_avconv $file -auto_conversion_filters $DEC_OPTS $2 -i "$input"
> "$ENC_OPTS -metadata title=lavftest" $4 -vcodec copy -acodec copy
> do_avconv_crc $file -auto_conversion_filters $DEC_OPTS -i
> $target_path/$file $3
> }
>
> diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak
> index ace8fa0b52..7a091de851 100644
> --- a/tests/fate/demux.mak
> +++ b/tests/fate/demux.mak
> @@ -14,7 +14,7 @@ FATE_SAMPLES_DEMUX-$(CONFIG_AEA_DEMUXER) += fate-aea-demux
> fate-aea-demux: CMD = crc -i $(TARGET_SAMPLES)/aea/chirp.aea -c:a copy
>
> FATE_SAMPLES_DEMUX-$(call DEMDEC, AV1, AV1) += fate-av1-annexb-demux
> -fate-av1-annexb-demux: CMD = framecrc -c:v av1 -i
> $(TARGET_SAMPLES)/av1/annexb.obu -c:v copy
> +fate-av1-annexb-demux: CMD = framecrc -c:v av1 -i
> $(TARGET_SAMPLES)/av1/annexb.obu -c:v copy -sar 1:1
>
> FATE_SAMPLES_DEMUX-$(CONFIG_AST_DEMUXER) += fate-ast
> fate-ast: CMD = crc -i $(TARGET_SAMPLES)/ast/demo11_02_partial.ast -c copy
> diff --git a/tests/fate/lavf-container.mak b/tests/fate/lavf-container.mak
> index 0d4a224601..047dd1d7f4 100644
> --- a/tests/fate/lavf-container.mak
> +++ b/tests/fate/lavf-container.mak
> @@ -86,8 +86,8 @@ FATE_LAVF_CONTAINER_FATE =
> $(FATE_LAVF_CONTAINER_FATE-yes:%=fate-lavf-fate-%)
> $(FATE_LAVF_CONTAINER_FATE): REF =
> $(SRC_PATH)/tests/ref/lavf-fate/$(@:fate-lavf-fate-%=%)
> $(FATE_LAVF_CONTAINER_FATE): $(AREF) $(VREF)
>
> -fate-lavf-fate-av1.mp4: CMD = lavf_container_fate
> "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy"
> -fate-lavf-fate-av1.mkv: CMD = lavf_container_fate
> "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy"
> +fate-lavf-fate-av1.mp4: CMD = lavf_container_fate
> "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" "-sar 1:1"
> +fate-lavf-fate-av1.mkv: CMD = lavf_container_fate
> "av1-test-vectors/av1-1-b8-05-mv.ivf" "" "-c:v copy" "-sar 1:1"
> fate-lavf-fate-evc.mp4: CMD = lavf_container_fate "evc/akiyo_cif.evc" ""
> "-c:v copy"
> fate-lavf-fate-h264.mp4: CMD = lavf_container_fate "h264/intra_refresh.h264"
> "" "-c:v copy"
> fate-lavf-fate-vp3.ogg: CMD = lavf_container_fate "vp3/coeff_level64.mkv"
> "-idct auto"
Will apply patches 4-15 as well as 22-25 tonight unless there are
objections.
- 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] avformat/mov: add interleaved_read option
Hi, On 9/10/2023 6:51 AM, Zhao Zhili wrote: > From: Zhao Zhili > > For bad interleaved files, manually interleave multiple tracks at the > demuxer level can trigger seeking back and forth, which can be > dramatically slow depending on the protocol. Demuxer level interleave > can be useless sometimes, e.g., reading mp4 via http and then > transcoding/remux to DASH. Disable this option when you don't need the > demuxer level interleave, and want to avoid the IO penalizes. > > This issue is well known. Two samples can be found at here > http://ffmpeg.org/pipermail/ffmpeg-devel/2022-December/304951.html Sorry for the slow reply. > +@item interleaved_read > +Interleaved read between multiple tracks, enabled by default. For bad > interleaved files, manually interleave multiple > +tracks at the demuxer level can trigger seeking back and forth, which can be > dramatically slow depending on the > +protocol. Disable this option when you don't need the demuxer level > interleave, and want to avoid the IO penalizes. I would write it with a description of what it does, rather than what disabling it would do, maybe something like: Interleave packets from multiple tracks at demuxer level. For badly interleaved files, this prevents playback issues caused by large gaps between packets in different tracks, as MOV/MP4 do not have packet placement requirements. However, this can cause excessive seeking on very badly interleaved files, due to seeking between tracks, so disabling it may prevent I/O issues, at the expense of playback. Again, apologies for forgetting to reply. - Derek ___ 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] avcodec/get_bits: Avoid reading multiple times in get_bits_long
Andreas Rheinhardt:
> Due to non-byte-alignment a read of 32 bits guarantees only
> 25 usable bits; therefore get_bits_long() (which is used to
> potentially read more than 25 bits) performs two reads in case
> it needs to read more than 25 bits and combines the result.
>
> Yet this is not necessary: One can just read 64 bits at a time
> to get 32 usable bits (57 would be possible). This commit does so.
>
> This reduced the size of .text by 30144B for GCC 11.4 and 5648B
> for Clang 14 (both with -O3).
>
> (get_bits_long() is a building block of show_bits_long()
> and get_ue_golomb_long(), so this patch affects these, too.)
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/get_bits.h | 38 +-
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/get_bits.h b/libavcodec/get_bits.h
> index 0594e104bb..f43bcae91e 100644
> --- a/libavcodec/get_bits.h
> +++ b/libavcodec/get_bits.h
> @@ -187,21 +187,28 @@ static inline unsigned int show_bits(GetBitContext *s,
> int n);
>
> #define CLOSE_READER(name, gb) (gb)->index = name ## _index
>
> +#define UPDATE_CACHE_BE_EXT(name, gb, bits, dst_bits) name ## _cache = \
> +AV_RB ## bits((gb)->buffer + (name ## _index >> 3)) << (name ## _index &
> 7) >> (bits - dst_bits)
> +
> +#define UPDATE_CACHE_LE_EXT(name, gb, bits, dst_bits) name ## _cache = \
> +(uint ## dst_bits ## _t)(AV_RL ## bits((gb)->buffer + (name ## _index >>
> 3)) >> (name ## _index & 7))
> +
> +/* Using these two macros ensures that 32 bits are available. */
> +# define UPDATE_CACHE_LE_32(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 64, 32)
> +
> +# define UPDATE_CACHE_BE_32(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 64, 32)
> +
> # ifdef LONG_BITSTREAM_READER
>
> -# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
> - AV_RL64((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
> +# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_32(name, (gb))
>
> -# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
> - AV_RB64((gb)->buffer + (name ## _index >> 3)) >> (32 - (name ## _index
> & 7))
> +# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_32(name, (gb))
>
> #else
>
> -# define UPDATE_CACHE_LE(name, gb) name ## _cache = \
> - AV_RL32((gb)->buffer + (name ## _index >> 3)) >> (name ## _index & 7)
> +# define UPDATE_CACHE_LE(name, gb) UPDATE_CACHE_LE_EXT(name, (gb), 32, 32)
>
> -# define UPDATE_CACHE_BE(name, gb) name ## _cache = \
> - AV_RB32((gb)->buffer + (name ## _index >> 3)) << (name ## _index & 7)
> +# define UPDATE_CACHE_BE(name, gb) UPDATE_CACHE_BE_EXT(name, (gb), 32, 32)
>
> #endif
>
> @@ -209,12 +216,14 @@ static inline unsigned int show_bits(GetBitContext *s,
> int n);
> #ifdef BITSTREAM_READER_LE
>
> # define UPDATE_CACHE(name, gb) UPDATE_CACHE_LE(name, gb)
> +# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_LE_32(name, (gb))
>
> # define SKIP_CACHE(name, gb, num) name ## _cache >>= (num)
>
> #else
>
> # define UPDATE_CACHE(name, gb) UPDATE_CACHE_BE(name, gb)
> +# define UPDATE_CACHE_32(name, gb) UPDATE_CACHE_BE_32(name, (gb))
>
> # define SKIP_CACHE(name, gb, num) name ## _cache <<= (num)
>
> @@ -414,15 +423,26 @@ static inline unsigned int get_bits_long(GetBitContext
> *s, int n)
> av_assert2(n>=0 && n<=32);
> if (!n) {
> return 0;
> -} else if (n <= MIN_CACHE_BITS) {
> +} else if ((!HAVE_FAST_64BIT || av_builtin_constant_p(n <=
> MIN_CACHE_BITS))
> + && n <= MIN_CACHE_BITS) {
> return get_bits(s, n);
> } else {
> +#if HAVE_FAST_64BIT
> +unsigned tmp;
> +OPEN_READER(re, s);
> +UPDATE_CACHE_32(re, s);
> +tmp = SHOW_UBITS(re, s, n);
> +LAST_SKIP_BITS(re, s, n);
> +CLOSE_READER(re, s);
> +return tmp;
> +#else
> #ifdef BITSTREAM_READER_LE
> unsigned ret = get_bits(s, 16);
> return ret | (get_bits(s, n - 16) << 16);
> #else
> unsigned ret = get_bits(s, 16) << (n - 16);
> return ret | get_bits(s, n - 16);
> +#endif
> #endif
> }
> }
Will apply this tonight unless there are objections.
- 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 16/22] avutil/imgutils: Constify some pointees
Andreas Rheinhardt:
> This is done immediately without waiting for the next major bump
> just as in 9546b3a1cbcd94e9107f85c8f1d2175efc6cf083 and
> 4eaaa38d3dfb8863a62f3646a62e4098b1c078d5.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> doc/APIchanges | 4
> libavutil/imgutils.c | 14 +++---
> libavutil/imgutils.h | 10 +-
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 963ad477bf..048232b2eb 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,10 @@ The last version increases of all libraries were on
> 2023-02-09
>
> API changes, most recent first:
>
> +2023-09-07 - xx - lavu 58.xx.100 - imgutils.h
> + Constify some pointees in av_image_copy(), av_image_copy_uc_from() and
> + av_image_fill_black().
> +
> 2023-09-07 - xx - lavf 60.xx.100 - avio.h
>Constify the buffer pointees in the write_packet and write_data_type
>callbacks of AVIOContext.
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 9ab5757cf6..da3812698e 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -378,8 +378,8 @@ void av_image_copy_plane(uint8_t *dst, int
> dst_linesize,
> image_copy_plane(dst, dst_linesize, src, src_linesize, bytewidth,
> height);
> }
>
> -static void image_copy(uint8_t *dst_data[4], const ptrdiff_t
> dst_linesizes[4],
> - const uint8_t *src_data[4], const ptrdiff_t
> src_linesizes[4],
> +static void image_copy(uint8_t *const dst_data[4], const ptrdiff_t
> dst_linesizes[4],
> + const uint8_t *const src_data[4], const ptrdiff_t
> src_linesizes[4],
> enum AVPixelFormat pix_fmt, int width, int height,
> void (*copy_plane)(uint8_t *, ptrdiff_t, const
> uint8_t *,
>ptrdiff_t, ptrdiff_t, int))
> @@ -419,8 +419,8 @@ static void image_copy(uint8_t *dst_data[4], const
> ptrdiff_t dst_linesizes[4],
> }
> }
>
> -void av_image_copy(uint8_t *dst_data[4], int dst_linesizes[4],
> - const uint8_t *src_data[4], const int src_linesizes[4],
> +void av_image_copy(uint8_t *const dst_data[4], const int dst_linesizes[4],
> + const uint8_t * const src_data[4], const int
> src_linesizes[4],
> enum AVPixelFormat pix_fmt, int width, int height)
> {
> ptrdiff_t dst_linesizes1[4], src_linesizes1[4];
> @@ -435,8 +435,8 @@ void av_image_copy(uint8_t *dst_data[4], int
> dst_linesizes[4],
> width, height, image_copy_plane);
> }
>
> -void av_image_copy_uc_from(uint8_t *dst_data[4], const ptrdiff_t
> dst_linesizes[4],
> - const uint8_t *src_data[4], const ptrdiff_t
> src_linesizes[4],
> +void av_image_copy_uc_from(uint8_t * const dst_data[4], const ptrdiff_t
> dst_linesizes[4],
> + const uint8_t * const src_data[4], const
> ptrdiff_t src_linesizes[4],
> enum AVPixelFormat pix_fmt, int width, int height)
> {
> image_copy(dst_data, dst_linesizes, src_data, src_linesizes, pix_fmt,
> @@ -579,7 +579,7 @@ static void memset_bytes(uint8_t *dst, size_t dst_size,
> uint8_t *clear,
> // if it's a subsampled packed format).
> #define MAX_BLOCK_SIZE 32
>
> -int av_image_fill_black(uint8_t *dst_data[4], const ptrdiff_t
> dst_linesize[4],
> +int av_image_fill_black(uint8_t * const dst_data[4], const ptrdiff_t
> dst_linesize[4],
> enum AVPixelFormat pix_fmt, enum AVColorRange range,
> int width, int height)
> {
> diff --git a/libavutil/imgutils.h b/libavutil/imgutils.h
> index e10ac14952..91312a72d3 100644
> --- a/libavutil/imgutils.h
> +++ b/libavutil/imgutils.h
> @@ -170,8 +170,8 @@ void av_image_copy_plane_uc_from(uint8_t *dst,
> ptrdiff_t dst_linesize,
> * @param width width of the image in pixels
> * @param heightheight of the image in pixels
> */
> -void av_image_copy(uint8_t *dst_data[4], int dst_linesizes[4],
> - const uint8_t *src_data[4], const int src_linesizes[4],
> +void av_image_copy(uint8_t * const dst_data[4], const int dst_linesizes[4],
> + const uint8_t * const src_data[4], const int
> src_linesizes[4],
> enum AVPixelFormat pix_fmt, int width, int height);
>
> /**
> @@ -188,8 +188,8 @@ void av_image_copy(uint8_t *dst_data[4], int
> dst_linesizes[4],
> * @note On x86, the linesizes currently need to be aligned to the cacheline
> * size (i.e. 64) to get improved performance.
> */
> -void av_image_copy_uc_from(uint8_t *dst_data[4], const ptrdiff_t
> dst_linesizes[4],
> - const uint8_t *src_data[4], const ptrdiff_t
> src_linesizes[4],
> +void av_image_copy_uc_from(uint8_t * const dst_data[4], const
> ptrdiff_t dst_linesizes[4],
> +
Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer
New version attached:
- fixed VP9 demuxing
- added support for alpha streams
- added support for subtitle streams
- numerous fixes and improvements
Can't get seeking to behave correctly with ADPCM_ADX audio streams.
Once one seek to start of file audio is no longer demuxed and video packets
are filling all queue.
From 5c32d4a9edb4f87ac2909909c732841bb6871e48 Mon Sep 17 00:00:00 2001
From: Paul B Mahol
Date: Tue, 5 Sep 2023 16:53:32 +0200
Subject: [PATCH] avformat: add CRI USM demuxer
Signed-off-by: Paul B Mahol
---
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/usmdec.c | 379 +++
3 files changed, 381 insertions(+)
create mode 100644 libavformat/usmdec.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index cc1b12360a..329055ccfd 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -588,6 +588,7 @@ OBJS-$(CONFIG_TTY_DEMUXER) += tty.o sauce.o
OBJS-$(CONFIG_TY_DEMUXER)+= ty.o
OBJS-$(CONFIG_TXD_DEMUXER) += txd.o
OBJS-$(CONFIG_UNCODEDFRAMECRC_MUXER) += uncodedframecrcenc.o framehash.o
+OBJS-$(CONFIG_USM_DEMUXER) += usmdec.o
OBJS-$(CONFIG_V210_DEMUXER) += rawvideodec.o
OBJS-$(CONFIG_V210X_DEMUXER) += rawvideodec.o
OBJS-$(CONFIG_VAG_DEMUXER) += vag.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index f4210e4932..d4b505a5a3 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -471,6 +471,7 @@ extern const AVInputFormat ff_txd_demuxer;
extern const AVInputFormat ff_tty_demuxer;
extern const AVInputFormat ff_ty_demuxer;
extern const FFOutputFormat ff_uncodedframecrc_muxer;
+extern const AVInputFormat ff_usm_demuxer;
extern const AVInputFormat ff_v210_demuxer;
extern const AVInputFormat ff_v210x_demuxer;
extern const AVInputFormat ff_vag_demuxer;
diff --git a/libavformat/usmdec.c b/libavformat/usmdec.c
new file mode 100644
index 00..f8bfe1a123
--- /dev/null
+++ b/libavformat/usmdec.c
@@ -0,0 +1,379 @@
+/*
+ * USM demuxer
+ * Copyright (c) 2023 Paul B Mahol
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/intreadwrite.h"
+#include "libavcodec/bytestream.h"
+
+#include "avformat.h"
+#include "internal.h"
+
+#define VIDEOI 0
+#define AUDIOI 1
+#define ALPHAI 2
+#define SUBTTI 3
+
+typedef struct USMChannel {
+int index;
+int used;
+} USMChannel;
+
+typedef struct USMDemuxContext {
+USMChannel ch[4][256];
+int nb_channels[4];
+uint8_t *header;
+unsigned header_size;
+} USMDemuxContext;
+
+static int usm_probe(const AVProbeData *p)
+{
+if (AV_RL32(p->buf) != MKTAG('C','R','I','D'))
+return 0;
+
+if (AV_RN32(p->buf + 4) == 0)
+return 0;
+
+return AVPROBE_SCORE_MAX / 3;
+}
+
+static int usm_read_header(AVFormatContext *s)
+{
+s->ctx_flags |= AVFMTCTX_NOHEADER;
+return 0;
+}
+
+static int parse_utf(AVFormatContext *s, AVIOContext *pb,
+ AVStream *st, AVCodecParameters *par, int ch_type,
+ uint32_t parent_chunk_size)
+{
+USMDemuxContext *usm = s->priv_data;
+GetByteContext gb, ugb, sgb;
+uint32_t chunk_type, chunk_size, offset;
+uint32_t unique_offset, string_offset;
+int nb_items, unique_size, nb_dictionaries;
+AVRational fps = { 0 };
+int type;
+
+chunk_type = avio_rb32(pb);
+chunk_size = avio_rb32(pb);
+
+if (chunk_type != MKBETAG('@','U','T','F'))
+return AVERROR_INVALIDDATA;
+
+if (!chunk_size || chunk_size >= parent_chunk_size)
+return AVERROR_INVALIDDATA;
+
+av_fast_malloc(&usm->header, &usm->header_size, chunk_size);
+if (!usm->header)
+return AVERROR(ENOMEM);
+
+if (avio_read(pb, usm->header, chunk_size) != chunk_size)
+return AVERROR_EOF;
+
+bytestream2_init(&gb, usm->header, chunk_size);
+ugb = gb;
+sgb = gb;
+unique_offset = bytestream2_get_be32(&gb);
+string_offset = bytestream2_get_be32(&gb);
+/*byte_offset =*/ bytestream2_get_be32(&gb);
+/*payload_name_offset =*/ bytestream2_get_be32(&gb);
+nb_items = bytestream2_get_be16(&gb);
+unique_size = bytestream2_get_be16(&gb);
+nb_dictiona
Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer
Paul B Mahol:
> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
>
> +chunk_type = avio_rb32(pb);
> +chunk_size = avio_rb32(pb);
You are not checking whether the chunk here exceeds its containing
>> chunk.
>
> +av_fast_malloc(&usm->header, &usm->header_size,
> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!usm->header)
> +return AVERROR(ENOMEM);
The bytestream2 API does not rely on the buffer being padded at all.
>
> +bytestream2_skip(&sgb, string_offset);
This is unnecessary, because you seek with an absolute offset lateron
anyway before using sgb.
>
> +bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
> +while (bytestream2_get_bytes_left(&sgb) > 0) {
> +key[n] = bytestream2_get_byte(&sgb);
> +if (!key[n])
> +break;
> +if (n >= sizeof(key) - 1)
> +break;
> +n++;
> +}
> +key[n] = '\0';
IMO this would be easier with strnlen(), avoiding sgb altogether.
You would of course need to explicitly check that you are not
overreading, but that is good practice anyway.
>
> +chunk_start = avio_tell(pb);
> +avio_skip(pb, 1);
> +payload_offset = avio_r8(pb);
> +padding_size = avio_rb16(pb);
> +stream_index = avio_r8(pb);
> +avio_skip(pb, 2);
> +payload_type = avio_r8(pb);
> +frame_time = avio_rb32(pb);
> +frame_rate = avio_rb32(pb);
> +avio_skip(pb, 8);
payload_offset and frame_time are set-but-unused; this might lead to
compiler warnings.
> +if (usm->ch[is_audio][stream_index].used == 1) {
> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
chunk_start);
> +
This is unnecessary: Unless we already had a read error, pkt_size is
chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
(Notice that in case padding_size is > 0, it will be part of the packet
with the current code; not sure if that is an issue.)
>
> +
> +avio_skip(pb, padding_size);
> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> +
Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET);
>>>
>>> But input might not be seekable.
>>>
>>
>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
>> SEEK_CUR)?
>>
>
> And? Do you know that SEEK_SET is different from SEEK_CUR with positive
> argument.
>
You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
an absolute seek.
- 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] avformat: add CRI USM demuxer
On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Paul B Mahol:
> > On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> >
> > +chunk_type = avio_rb32(pb);
> > +chunk_size = avio_rb32(pb);
>
> You are not checking whether the chunk here exceeds its containing
> >> chunk.
>
> >
> > +av_fast_malloc(&usm->header, &usm->header_size,
> > + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> > +if (!usm->header)
> > +return AVERROR(ENOMEM);
>
> The bytestream2 API does not rely on the buffer being padded at all.
>
> >
> > +bytestream2_skip(&sgb, string_offset);
>
> This is unnecessary, because you seek with an absolute offset lateron
> anyway before using sgb.
>
> >
> > +bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
> > +while (bytestream2_get_bytes_left(&sgb) > 0) {
> > +key[n] = bytestream2_get_byte(&sgb);
> > +if (!key[n])
> > +break;
> > +if (n >= sizeof(key) - 1)
> > +break;
> > +n++;
> > +}
> > +key[n] = '\0';
>
> IMO this would be easier with strnlen(), avoiding sgb altogether.
> You would of course need to explicitly check that you are not
> overreading, but that is good practice anyway.
>
> >
> > +chunk_start = avio_tell(pb);
> > +avio_skip(pb, 1);
> > +payload_offset = avio_r8(pb);
> > +padding_size = avio_rb16(pb);
> > +stream_index = avio_r8(pb);
> > +avio_skip(pb, 2);
> > +payload_type = avio_r8(pb);
> > +frame_time = avio_rb32(pb);
> > +frame_rate = avio_rb32(pb);
> > +avio_skip(pb, 8);
>
> payload_offset and frame_time are set-but-unused; this might lead to
> compiler warnings.
>
> > +if (usm->ch[is_audio][stream_index].used == 1) {
> > +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
> chunk_start);
> > +
>
> This is unnecessary: Unless we already had a read error, pkt_size is
> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>
> (Notice that in case padding_size is > 0, it will be part of the
> packet
> with the current code; not sure if that is an issue.)
>
> >
> > +
> > +avio_skip(pb, padding_size);
> > +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> > +
>
> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET);
>
> >>>
> >>> But input might not be seekable.
> >>>
> >>
> >> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
> >> SEEK_CUR)?
> >>
> >
> > And? Do you know that SEEK_SET is different from SEEK_CUR with positive
> > argument.
> >
>
> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
> an absolute seek.
>
Nope, I skip data only, not seeking backward ever.
>
> - 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".
>
___
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: add CRI USM demuxer
Paul B Mahol:
> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>>
>>> +chunk_type = avio_rb32(pb);
>>> +chunk_size = avio_rb32(pb);
>>
>> You are not checking whether the chunk here exceeds its containing
chunk.
>>
>>>
>>> +av_fast_malloc(&usm->header, &usm->header_size,
>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
>>> +if (!usm->header)
>>> +return AVERROR(ENOMEM);
>>
>> The bytestream2 API does not rely on the buffer being padded at all.
>>
>>>
>>> +bytestream2_skip(&sgb, string_offset);
>>
>> This is unnecessary, because you seek with an absolute offset lateron
>> anyway before using sgb.
>>
>>>
>>> +bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
>>> +while (bytestream2_get_bytes_left(&sgb) > 0) {
>>> +key[n] = bytestream2_get_byte(&sgb);
>>> +if (!key[n])
>>> +break;
>>> +if (n >= sizeof(key) - 1)
>>> +break;
>>> +n++;
>>> +}
>>> +key[n] = '\0';
>>
>> IMO this would be easier with strnlen(), avoiding sgb altogether.
>> You would of course need to explicitly check that you are not
>> overreading, but that is good practice anyway.
>>
>>>
>>> +chunk_start = avio_tell(pb);
>>> +avio_skip(pb, 1);
>>> +payload_offset = avio_r8(pb);
>>> +padding_size = avio_rb16(pb);
>>> +stream_index = avio_r8(pb);
>>> +avio_skip(pb, 2);
>>> +payload_type = avio_r8(pb);
>>> +frame_time = avio_rb32(pb);
>>> +frame_rate = avio_rb32(pb);
>>> +avio_skip(pb, 8);
>>
>> payload_offset and frame_time are set-but-unused; this might lead to
>> compiler warnings.
>>
>>> +if (usm->ch[is_audio][stream_index].used == 1) {
>>> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
>> chunk_start);
>>> +
>>
>> This is unnecessary: Unless we already had a read error, pkt_size is
>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>>
>> (Notice that in case padding_size is > 0, it will be part of the
>> packet
>> with the current code; not sure if that is an issue.)
>>
>>>
>>> +
>>> +avio_skip(pb, padding_size);
>>> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
>>> +
>>
>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET);
>>
>
> But input might not be seekable.
>
And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
SEEK_CUR)?
>>>
>>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive
>>> argument.
>>>
>>
>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
>> an absolute seek.
>>
>
> Nope, I skip data only, not seeking backward ever.
>
avio_seek() internally translates any avio_seek() with SEEK_CUR to an
absolute seek (as required by the seek callbacks) and does not care
about whether it is seeking forward or backward.
- 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] avformat: add CRI USM demuxer
On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>>
> >>> +chunk_type = avio_rb32(pb);
> >>> +chunk_size = avio_rb32(pb);
> >>
> >> You are not checking whether the chunk here exceeds its containing
> chunk.
> >>
> >>>
> >>> +av_fast_malloc(&usm->header, &usm->header_size,
> >>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> >>> +if (!usm->header)
> >>> +return AVERROR(ENOMEM);
> >>
> >> The bytestream2 API does not rely on the buffer being padded at all.
> >>
> >>>
> >>> +bytestream2_skip(&sgb, string_offset);
> >>
> >> This is unnecessary, because you seek with an absolute offset
> lateron
> >> anyway before using sgb.
> >>
> >>>
> >>> +bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
> >>> +while (bytestream2_get_bytes_left(&sgb) > 0) {
> >>> +key[n] = bytestream2_get_byte(&sgb);
> >>> +if (!key[n])
> >>> +break;
> >>> +if (n >= sizeof(key) - 1)
> >>> +break;
> >>> +n++;
> >>> +}
> >>> +key[n] = '\0';
> >>
> >> IMO this would be easier with strnlen(), avoiding sgb altogether.
> >> You would of course need to explicitly check that you are not
> >> overreading, but that is good practice anyway.
> >>
> >>>
> >>> +chunk_start = avio_tell(pb);
> >>> +avio_skip(pb, 1);
> >>> +payload_offset = avio_r8(pb);
> >>> +padding_size = avio_rb16(pb);
> >>> +stream_index = avio_r8(pb);
> >>> +avio_skip(pb, 2);
> >>> +payload_type = avio_r8(pb);
> >>> +frame_time = avio_rb32(pb);
> >>> +frame_rate = avio_rb32(pb);
> >>> +avio_skip(pb, 8);
> >>
> >> payload_offset and frame_time are set-but-unused; this might lead to
> >> compiler warnings.
> >>
> >>> +if (usm->ch[is_audio][stream_index].used == 1) {
> >>> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
> >> chunk_start);
> >>> +
> >>
> >> This is unnecessary: Unless we already had a read error, pkt_size is
> >> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
> >>
> >> (Notice that in case padding_size is > 0, it will be part of the
> >> packet
> >> with the current code; not sure if that is an issue.)
> >>
> >>>
> >>> +
> >>> +avio_skip(pb, padding_size);
> >>> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> >>> +
> >>
> >> Simpler to just use avio_seek(pb, chunk_start + chunk_size,
> SEEK_SET);
> >>
> >
> > But input might not be seekable.
> >
>
> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
> SEEK_CUR)?
>
> >>>
> >>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive
> >>> argument.
> >>>
> >>
> >> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
> >> an absolute seek.
> >>
> >
> > Nope, I skip data only, not seeking backward ever.
> >
>
> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
> absolute seek (as required by the seek callbacks) and does not care
> about whether it is seeking forward or backward.
>
Irrelevant. Seeking needs enough cache to work on non-seekable input.
>
> - 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".
>
___
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: add CRI USM demuxer
Paul B Mahol:
> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
>
> +chunk_type = avio_rb32(pb);
> +chunk_size = avio_rb32(pb);
You are not checking whether the chunk here exceeds its containing
>> chunk.
>
> +av_fast_malloc(&usm->header, &usm->header_size,
> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!usm->header)
> +return AVERROR(ENOMEM);
The bytestream2 API does not rely on the buffer being padded at all.
>
> +bytestream2_skip(&sgb, string_offset);
This is unnecessary, because you seek with an absolute offset
>> lateron
anyway before using sgb.
>
> +bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
> +while (bytestream2_get_bytes_left(&sgb) > 0) {
> +key[n] = bytestream2_get_byte(&sgb);
> +if (!key[n])
> +break;
> +if (n >= sizeof(key) - 1)
> +break;
> +n++;
> +}
> +key[n] = '\0';
IMO this would be easier with strnlen(), avoiding sgb altogether.
You would of course need to explicitly check that you are not
overreading, but that is good practice anyway.
>
> +chunk_start = avio_tell(pb);
> +avio_skip(pb, 1);
> +payload_offset = avio_r8(pb);
> +padding_size = avio_rb16(pb);
> +stream_index = avio_r8(pb);
> +avio_skip(pb, 2);
> +payload_type = avio_r8(pb);
> +frame_time = avio_rb32(pb);
> +frame_rate = avio_rb32(pb);
> +avio_skip(pb, 8);
payload_offset and frame_time are set-but-unused; this might lead to
compiler warnings.
> +if (usm->ch[is_audio][stream_index].used == 1) {
> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
chunk_start);
> +
This is unnecessary: Unless we already had a read error, pkt_size is
chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
(Notice that in case padding_size is > 0, it will be part of the
packet
with the current code; not sure if that is an issue.)
>
> +
> +avio_skip(pb, padding_size);
> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> +
Simpler to just use avio_seek(pb, chunk_start + chunk_size,
>> SEEK_SET);
>>>
>>> But input might not be seekable.
>>>
>>
>> And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
>> SEEK_CUR)?
>>
>
> And? Do you know that SEEK_SET is different from SEEK_CUR with positive
> argument.
>
You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
an absolute seek.
>>>
>>> Nope, I skip data only, not seeking backward ever.
>>>
>>
>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
>> absolute seek (as required by the seek callbacks) and does not care
>> about whether it is seeking forward or backward.
>>
>
> Irrelevant. Seeking needs enough cache to work on non-seekable input.
>
Given that avio_skip() is just a wrapper around avio_seek(), the
behaviour will not change even when the input is non-seekable.
- 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] avformat: add CRI USM demuxer
On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> >
> > +chunk_type = avio_rb32(pb);
> > +chunk_size = avio_rb32(pb);
>
> You are not checking whether the chunk here exceeds its containing
> >> chunk.
>
> >
> > +av_fast_malloc(&usm->header, &usm->header_size,
> > + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> > +if (!usm->header)
> > +return AVERROR(ENOMEM);
>
> The bytestream2 API does not rely on the buffer being padded at
> all.
>
> >
> > +bytestream2_skip(&sgb, string_offset);
>
> This is unnecessary, because you seek with an absolute offset
> >> lateron
> anyway before using sgb.
>
> >
> > +bytestream2_seek(&sgb, string_offset + offset,
> SEEK_SET);
> > +while (bytestream2_get_bytes_left(&sgb) > 0) {
> > +key[n] = bytestream2_get_byte(&sgb);
> > +if (!key[n])
> > +break;
> > +if (n >= sizeof(key) - 1)
> > +break;
> > +n++;
> > +}
> > +key[n] = '\0';
>
> IMO this would be easier with strnlen(), avoiding sgb altogether.
> You would of course need to explicitly check that you are not
> overreading, but that is good practice anyway.
>
> >
> > +chunk_start = avio_tell(pb);
> > +avio_skip(pb, 1);
> > +payload_offset = avio_r8(pb);
> > +padding_size = avio_rb16(pb);
> > +stream_index = avio_r8(pb);
> > +avio_skip(pb, 2);
> > +payload_type = avio_r8(pb);
> > +frame_time = avio_rb32(pb);
> > +frame_rate = avio_rb32(pb);
> > +avio_skip(pb, 8);
>
> payload_offset and frame_time are set-but-unused; this might lead
> to
> compiler warnings.
>
> > +if (usm->ch[is_audio][stream_index].used == 1) {
> > +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
> chunk_start);
> > +
>
> This is unnecessary: Unless we already had a read error, pkt_size
> is
> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>
> (Notice that in case padding_size is > 0, it will be part of the
> packet
> with the current code; not sure if that is an issue.)
>
> >
> > +
> > +avio_skip(pb, padding_size);
> > +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> > +
>
> Simpler to just use avio_seek(pb, chunk_start + chunk_size,
> >> SEEK_SET);
>
> >>>
> >>> But input might not be seekable.
> >>>
> >>
> >> And? You know that avio_skip(pb, offset) is just avio_seek(pb,
> offset,
> >> SEEK_CUR)?
> >>
> >
> > And? Do you know that SEEK_SET is different from SEEK_CUR with
> positive
> > argument.
> >
>
> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
> an absolute seek.
>
> >>>
> >>> Nope, I skip data only, not seeking backward ever.
> >>>
> >>
> >> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
> >> absolute seek (as required by the seek callbacks) and does not care
> >> about whether it is seeking forward or backward.
> >>
> >
> > Irrelevant. Seeking needs enough cache to work on non-seekable input.
> >
>
> Given that avio_skip() is just a wrapper around avio_seek(), the
> behaviour will not change even when the input is non-seekable.
>
What you are trying to sell?
That seeking works with unseekable input?
>
> - 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".
>
___
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/3] avcodec/av1dec: Pass AVCodecContext* as logctx in get_sw_pixel_format()
It indicates to the reader that said function does not modify
any state.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/av1dec.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 8f9c2dfefb..8f6c4f732e 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -440,7 +440,7 @@ static int get_tiles_info(AVCodecContext *avctx, const
AV1RawTileGroup *tile_gro
}
-static enum AVPixelFormat get_sw_pixel_format(AVCodecContext *avctx,
+static enum AVPixelFormat get_sw_pixel_format(void *logctx,
const AV1RawSequenceHeader *seq)
{
uint8_t bit_depth;
@@ -451,7 +451,7 @@ static enum AVPixelFormat
get_sw_pixel_format(AVCodecContext *avctx,
else if (seq->seq_profile <= 2)
bit_depth = seq->color_config.high_bitdepth ? 10 : 8;
else {
-av_log(avctx, AV_LOG_ERROR,
+av_log(logctx, AV_LOG_ERROR,
"Unknown AV1 profile %d.\n", seq->seq_profile);
return -1;
}
@@ -467,7 +467,7 @@ static enum AVPixelFormat
get_sw_pixel_format(AVCodecContext *avctx,
else if (bit_depth == 12)
pix_fmt = AV_PIX_FMT_YUV444P12;
else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
} else if (seq->color_config.subsampling_x == 1 &&
seq->color_config.subsampling_y == 0) {
if (bit_depth == 8)
@@ -477,7 +477,7 @@ static enum AVPixelFormat
get_sw_pixel_format(AVCodecContext *avctx,
else if (bit_depth == 12)
pix_fmt = AV_PIX_FMT_YUV422P12;
else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
} else if (seq->color_config.subsampling_x == 1 &&
seq->color_config.subsampling_y == 1) {
if (bit_depth == 8)
@@ -487,7 +487,7 @@ static enum AVPixelFormat
get_sw_pixel_format(AVCodecContext *avctx,
else if (bit_depth == 12)
pix_fmt = AV_PIX_FMT_YUV420P12;
else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
}
} else {
if (bit_depth == 8)
@@ -497,7 +497,7 @@ static enum AVPixelFormat
get_sw_pixel_format(AVCodecContext *avctx,
else if (bit_depth == 12)
pix_fmt = AV_PIX_FMT_GRAY12;
else
-av_log(avctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
+av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
}
return pix_fmt;
--
2.34.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 2/3] avcodec/av1dec: Don't rely on AV_PIX_FMT_NONE == -1
Since fb548fba04193a418f118d21b261ba05db4f480b,
this return -1 is in a function returning enum AVPixelFormat
whose caller checks for AV_PIX_FMT_NONE for failure.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/av1dec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 8f6c4f732e..c523c457ec 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -453,7 +453,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
else {
av_log(logctx, AV_LOG_ERROR,
"Unknown AV1 profile %d.\n", seq->seq_profile);
-return -1;
+return AV_PIX_FMT_NONE;
}
if (!seq->color_config.mono_chrome) {
--
2.34.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: add CRI USM demuxer
Paul B Mahol:
> On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>>
>>> +chunk_type = avio_rb32(pb);
>>> +chunk_size = avio_rb32(pb);
>>
>> You are not checking whether the chunk here exceeds its containing
chunk.
>>
>>>
>>> +av_fast_malloc(&usm->header, &usm->header_size,
>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
>>> +if (!usm->header)
>>> +return AVERROR(ENOMEM);
>>
>> The bytestream2 API does not rely on the buffer being padded at
>> all.
>>
>>>
>>> +bytestream2_skip(&sgb, string_offset);
>>
>> This is unnecessary, because you seek with an absolute offset
lateron
>> anyway before using sgb.
>>
>>>
>>> +bytestream2_seek(&sgb, string_offset + offset,
>> SEEK_SET);
>>> +while (bytestream2_get_bytes_left(&sgb) > 0) {
>>> +key[n] = bytestream2_get_byte(&sgb);
>>> +if (!key[n])
>>> +break;
>>> +if (n >= sizeof(key) - 1)
>>> +break;
>>> +n++;
>>> +}
>>> +key[n] = '\0';
>>
>> IMO this would be easier with strnlen(), avoiding sgb altogether.
>> You would of course need to explicitly check that you are not
>> overreading, but that is good practice anyway.
>>
>>>
>>> +chunk_start = avio_tell(pb);
>>> +avio_skip(pb, 1);
>>> +payload_offset = avio_r8(pb);
>>> +padding_size = avio_rb16(pb);
>>> +stream_index = avio_r8(pb);
>>> +avio_skip(pb, 2);
>>> +payload_type = avio_r8(pb);
>>> +frame_time = avio_rb32(pb);
>>> +frame_rate = avio_rb32(pb);
>>> +avio_skip(pb, 8);
>>
>> payload_offset and frame_time are set-but-unused; this might lead
>> to
>> compiler warnings.
>>
>>> +if (usm->ch[is_audio][stream_index].used == 1) {
>>> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
>> chunk_start);
>>> +
>>
>> This is unnecessary: Unless we already had a read error, pkt_size
>> is
>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>>
>> (Notice that in case padding_size is > 0, it will be part of the
>> packet
>> with the current code; not sure if that is an issue.)
>>
>>>
>>> +
>>> +avio_skip(pb, padding_size);
>>> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
>>> +
>>
>> Simpler to just use avio_seek(pb, chunk_start + chunk_size,
SEEK_SET);
>>
>
> But input might not be seekable.
>
And? You know that avio_skip(pb, offset) is just avio_seek(pb,
>> offset,
SEEK_CUR)?
>>>
>>> And? Do you know that SEEK_SET is different from SEEK_CUR with
>> positive
>>> argument.
>>>
>>
>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
>> an absolute seek.
>>
>
> Nope, I skip data only, not seeking backward ever.
>
avio_seek() internally translates any avio_seek() with SEEK_CUR to an
absolute seek (as required by the seek callbacks) and does not care
about whether it is seeking forward or backward.
>>>
>>> Irrelevant. Seeking needs enough cache to work on non-seekable input.
>>>
>>
>> Given that avio_skip() is just a wrapper around avio_seek(), the
>> behaviour will not change even when the input is non-seekable.
>>
>
> What you are trying to sell?
>
> That seeking works with unseekable input?
>
Seeking forward works for unseekable input by reading data and
discarding it (that is how avio_seek() works). But actually we don't
need that, all we need is that avio_skip() is just a wrapper around
avio_seek(), so that the two forms are equivalent.
- 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 su
[FFmpeg-devel] [PATCH 3/3] avcodec/thread: Remove ff_thread_get_format define
Unnecessary since FF_API_THREAD_SAFE_CALLBACKS is no more. Signed-off-by: Andreas Rheinhardt --- libavcodec/av1dec.c | 2 +- libavcodec/h264_slice.c | 2 +- libavcodec/hevcdec.c| 2 +- libavcodec/mpeg12dec.c | 2 +- libavcodec/proresdec2.c | 2 +- libavcodec/thread.h | 2 -- libavcodec/vp9.c| 2 +- 7 files changed, 6 insertions(+), 8 deletions(-) diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index c523c457ec..39ccad5bf6 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -613,7 +613,7 @@ static int get_pixel_format(AVCodecContext *avctx) *fmtp++ = pix_fmt; *fmtp = AV_PIX_FMT_NONE; -ret = ff_thread_get_format(avctx, pix_fmts); +ret = ff_get_format(avctx, pix_fmts); /** * check if the HW accel is inited correctly. If not, return un-implemented. diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 6cd7bb8fe7..f3af345c99 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -922,7 +922,7 @@ static enum AVPixelFormat get_pixel_format(H264Context *h, int force_callback) for (int i = 0; pix_fmts[i] != AV_PIX_FMT_NONE; i++) if (pix_fmts[i] == h->avctx->pix_fmt && !force_callback) return pix_fmts[i]; -return ff_thread_get_format(h->avctx, pix_fmts); +return ff_get_format(h->avctx, pix_fmts); } /* export coded and cropped frame dimensions to AVCodecContext */ diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c index ad22d0a30c..81b9c5e089 100644 --- a/libavcodec/hevcdec.c +++ b/libavcodec/hevcdec.c @@ -521,7 +521,7 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps) *fmt++ = sps->pix_fmt; *fmt = AV_PIX_FMT_NONE; -return ff_thread_get_format(s->avctx, pix_fmts); +return ff_get_format(s->avctx, pix_fmts); } static int set_sps(HEVCContext *s, const HEVCSPS *sps, diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index 1accd07e9e..677360f954 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -1168,7 +1168,7 @@ static enum AVPixelFormat mpeg_get_pixelformat(AVCodecContext *avctx) else pix_fmts = mpeg12_pixfmt_list_444; -return ff_thread_get_format(avctx, pix_fmts); +return ff_get_format(avctx, pix_fmts); } /* Call this function when we know all parameters. diff --git a/libavcodec/proresdec2.c b/libavcodec/proresdec2.c index 9297860946..def58b2604 100644 --- a/libavcodec/proresdec2.c +++ b/libavcodec/proresdec2.c @@ -278,7 +278,7 @@ static int decode_frame_header(ProresContext *ctx, const uint8_t *buf, *fmtp++ = ctx->pix_fmt; *fmtp = AV_PIX_FMT_NONE; -if ((ret = ff_thread_get_format(avctx, pix_fmts)) < 0) +if ((ret = ff_get_format(avctx, pix_fmts)) < 0) return ret; avctx->pix_fmt = ret; diff --git a/libavcodec/thread.h b/libavcodec/thread.h index 88a14cfeb1..2c8c0cdb16 100644 --- a/libavcodec/thread.h +++ b/libavcodec/thread.h @@ -62,8 +62,6 @@ int ff_thread_decode_frame(AVCodecContext *avctx, AVFrame *picture, */ void ff_thread_finish_setup(AVCodecContext *avctx); -#define ff_thread_get_format ff_get_format - /** * Wrapper around get_buffer() for frame-multithreaded codecs. * Call this function instead of ff_get_buffer(f). diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c index 89f7549ef0..3cc27aa812 100644 --- a/libavcodec/vp9.c +++ b/libavcodec/vp9.c @@ -250,7 +250,7 @@ static int update_size(AVCodecContext *avctx, int w, int h) *fmtp++ = s->pix_fmt; *fmtp = AV_PIX_FMT_NONE; -ret = ff_thread_get_format(avctx, pix_fmts); +ret = ff_get_format(avctx, pix_fmts); if (ret < 0) return ret; -- 2.34.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: add CRI USM demuxer
On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>>
> >>> +chunk_type = avio_rb32(pb);
> >>> +chunk_size = avio_rb32(pb);
> >>
> >> You are not checking whether the chunk here exceeds its
> containing
> chunk.
> >>
> >>>
> >>> +av_fast_malloc(&usm->header, &usm->header_size,
> >>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> >>> +if (!usm->header)
> >>> +return AVERROR(ENOMEM);
> >>
> >> The bytestream2 API does not rely on the buffer being padded at
> >> all.
> >>
> >>>
> >>> +bytestream2_skip(&sgb, string_offset);
> >>
> >> This is unnecessary, because you seek with an absolute offset
> lateron
> >> anyway before using sgb.
> >>
> >>>
> >>> +bytestream2_seek(&sgb, string_offset + offset,
> >> SEEK_SET);
> >>> +while (bytestream2_get_bytes_left(&sgb) > 0) {
> >>> +key[n] = bytestream2_get_byte(&sgb);
> >>> +if (!key[n])
> >>> +break;
> >>> +if (n >= sizeof(key) - 1)
> >>> +break;
> >>> +n++;
> >>> +}
> >>> +key[n] = '\0';
> >>
> >> IMO this would be easier with strnlen(), avoiding sgb
> altogether.
> >> You would of course need to explicitly check that you are not
> >> overreading, but that is good practice anyway.
> >>
> >>>
> >>> +chunk_start = avio_tell(pb);
> >>> +avio_skip(pb, 1);
> >>> +payload_offset = avio_r8(pb);
> >>> +padding_size = avio_rb16(pb);
> >>> +stream_index = avio_r8(pb);
> >>> +avio_skip(pb, 2);
> >>> +payload_type = avio_r8(pb);
> >>> +frame_time = avio_rb32(pb);
> >>> +frame_rate = avio_rb32(pb);
> >>> +avio_skip(pb, 8);
> >>
> >> payload_offset and frame_time are set-but-unused; this might
> lead
> >> to
> >> compiler warnings.
> >>
> >>> +if (usm->ch[is_audio][stream_index].used == 1) {
> >>> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
> >> chunk_start);
> >>> +
> >>
> >> This is unnecessary: Unless we already had a read error,
> pkt_size
> >> is
> >> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
> >>
> >> (Notice that in case padding_size is > 0, it will be part of the
> >> packet
> >> with the current code; not sure if that is an issue.)
> >>
> >>>
> >>> +
> >>> +avio_skip(pb, padding_size);
> >>> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> >>> +
> >>
> >> Simpler to just use avio_seek(pb, chunk_start + chunk_size,
> SEEK_SET);
> >>
> >
> > But input might not be seekable.
> >
>
> And? You know that avio_skip(pb, offset) is just avio_seek(pb,
> >> offset,
> SEEK_CUR)?
>
> >>>
> >>> And? Do you know that SEEK_SET is different from SEEK_CUR with
> >> positive
> >>> argument.
> >>>
> >>
> >> You are using SEEK_CUR with -avio_tell(pb), which effectively makes
> it
> >> an absolute seek.
> >>
> >
> > Nope, I skip data only, not seeking backward ever.
> >
>
> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
> absolute seek (as required by the seek callbacks) and does not care
> about whether it is seeking forward or backward.
>
> >>>
> >>> Irrelevant. Seeking needs enough cache to work on non-seekable input.
> >>>
> >>
> >> Given that avio_skip() is just a wrapper around avio_seek(), the
> >> behaviour will not change even when the input is non-seekable.
> >>
> >
> > What you are trying to sell?
> >
> > That seeking works with unseekable input?
> >
>
> Seeking forward works for unseekable input by reading data and
> discarding it (that is how avio_seek() works). But act
Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer
Paul B Mahol:
> On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
Paul B Mahol:
>
> +chunk_type = avio_rb32(pb);
> +chunk_size = avio_rb32(pb);
You are not checking whether the chunk here exceeds its
>> containing
>> chunk.
>
> +av_fast_malloc(&usm->header, &usm->header_size,
> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!usm->header)
> +return AVERROR(ENOMEM);
The bytestream2 API does not rely on the buffer being padded at
all.
>
> +bytestream2_skip(&sgb, string_offset);
This is unnecessary, because you seek with an absolute offset
>> lateron
anyway before using sgb.
>
> +bytestream2_seek(&sgb, string_offset + offset,
SEEK_SET);
> +while (bytestream2_get_bytes_left(&sgb) > 0) {
> +key[n] = bytestream2_get_byte(&sgb);
> +if (!key[n])
> +break;
> +if (n >= sizeof(key) - 1)
> +break;
> +n++;
> +}
> +key[n] = '\0';
IMO this would be easier with strnlen(), avoiding sgb
>> altogether.
You would of course need to explicitly check that you are not
overreading, but that is good practice anyway.
>
> +chunk_start = avio_tell(pb);
> +avio_skip(pb, 1);
> +payload_offset = avio_r8(pb);
> +padding_size = avio_rb16(pb);
> +stream_index = avio_r8(pb);
> +avio_skip(pb, 2);
> +payload_type = avio_r8(pb);
> +frame_time = avio_rb32(pb);
> +frame_rate = avio_rb32(pb);
> +avio_skip(pb, 8);
payload_offset and frame_time are set-but-unused; this might
>> lead
to
compiler warnings.
> +if (usm->ch[is_audio][stream_index].used == 1) {
> +uint32_t pkt_size = chunk_size - (avio_tell(pb) -
chunk_start);
> +
This is unnecessary: Unless we already had a read error,
>> pkt_size
is
chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
(Notice that in case padding_size is > 0, it will be part of the
packet
with the current code; not sure if that is an issue.)
>
> +
> +avio_skip(pb, padding_size);
> +avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
> +
Simpler to just use avio_seek(pb, chunk_start + chunk_size,
>> SEEK_SET);
>>>
>>> But input might not be seekable.
>>>
>>
>> And? You know that avio_skip(pb, offset) is just avio_seek(pb,
offset,
>> SEEK_CUR)?
>>
>
> And? Do you know that SEEK_SET is different from SEEK_CUR with
positive
> argument.
>
You are using SEEK_CUR with -avio_tell(pb), which effectively makes
>> it
an absolute seek.
>>>
>>> Nope, I skip data only, not seeking backward ever.
>>>
>>
>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
>> absolute seek (as required by the seek callbacks) and does not care
>> about whether it is seeking forward or backward.
>>
>
> Irrelevant. Seeking needs enough cache to work on non-seekable input.
>
Given that avio_skip() is just a wrapper around avio_seek(), the
behaviour will not change even when the input is non-seekable.
>>>
>>> What you are trying to sell?
>>>
>>> That seeking works with unseekable input?
>>>
>>
>> Seeking forward works for unseekable input by reading data and
>> discarding it (that is ho
Re: [FFmpeg-devel] [PATCH] avformat: add CRI USM demuxer
On Sun, Sep 10, 2023 at 3:13 PM Andreas Rheinhardt <
andreas.rheinha...@outlook.com> wrote:
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 2:54 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Sun, Sep 10, 2023 at 2:18 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> > On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> >> Paul B Mahol:
> >>> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> >>> andreas.rheinha...@outlook.com> wrote:
> >>>
> Paul B Mahol:
> >
> > +chunk_type = avio_rb32(pb);
> > +chunk_size = avio_rb32(pb);
>
> You are not checking whether the chunk here exceeds its
> >> containing
> >> chunk.
>
> >
> > +av_fast_malloc(&usm->header, &usm->header_size,
> > + chunk_size +
> AV_INPUT_BUFFER_PADDING_SIZE);
> > +if (!usm->header)
> > +return AVERROR(ENOMEM);
>
> The bytestream2 API does not rely on the buffer being padded
> at
> all.
>
> >
> > +bytestream2_skip(&sgb, string_offset);
>
> This is unnecessary, because you seek with an absolute offset
> >> lateron
> anyway before using sgb.
>
> >
> > +bytestream2_seek(&sgb, string_offset + offset,
> SEEK_SET);
> > +while (bytestream2_get_bytes_left(&sgb) > 0) {
> > +key[n] = bytestream2_get_byte(&sgb);
> > +if (!key[n])
> > +break;
> > +if (n >= sizeof(key) - 1)
> > +break;
> > +n++;
> > +}
> > +key[n] = '\0';
>
> IMO this would be easier with strnlen(), avoiding sgb
> >> altogether.
> You would of course need to explicitly check that you are not
> overreading, but that is good practice anyway.
>
> >
> > +chunk_start = avio_tell(pb);
> > +avio_skip(pb, 1);
> > +payload_offset = avio_r8(pb);
> > +padding_size = avio_rb16(pb);
> > +stream_index = avio_r8(pb);
> > +avio_skip(pb, 2);
> > +payload_type = avio_r8(pb);
> > +frame_time = avio_rb32(pb);
> > +frame_rate = avio_rb32(pb);
> > +avio_skip(pb, 8);
>
> payload_offset and frame_time are set-but-unused; this might
> >> lead
> to
> compiler warnings.
>
> > +if (usm->ch[is_audio][stream_index].used == 1) {
> > +uint32_t pkt_size = chunk_size - (avio_tell(pb)
> -
> chunk_start);
> > +
>
> This is unnecessary: Unless we already had a read error,
> >> pkt_size
> is
> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>
> (Notice that in case padding_size is > 0, it will be part of
> the
> packet
> with the current code; not sure if that is an issue.)
>
> >
> > +
> > +avio_skip(pb, padding_size);
> > +avio_skip(pb, chunk_size - (avio_tell(pb) -
> chunk_start));
> > +
>
> Simpler to just use avio_seek(pb, chunk_start + chunk_size,
> >> SEEK_SET);
>
> >>>
> >>> But input might not be seekable.
> >>>
> >>
> >> And? You know that avio_skip(pb, offset) is just avio_seek(pb,
> offset,
> >> SEEK_CUR)?
> >>
> >
> > And? Do you know that SEEK_SET is different from SEEK_CUR with
> positive
> > argument.
> >
>
> You are using SEEK_CUR with -avio_tell(pb), which effectively
> makes
> >> it
> an absolute seek.
>
> >>>
> >>> Nope, I skip data only, not seeking backward ever.
> >>>
> >>
> >> avio_seek() internally translates any avio_seek() with SEEK_CUR to
> an
> >> absolute seek (as required by the seek callbacks) and does not care
> >> about whether it is seeking forward or backward.
> >>
> >
> > Irrelevant. Seeking needs enough cac
Re: [FFmpeg-devel] [PATCH] avcodec/cscd: Check for CamStudio Lossless Codec 1.0 behavior in end check
On Wed, Mar 08, 2023 at 11:43:38PM +0100, Michael Niedermayer wrote: > Alternatively the check could be simply made more tolerant > Fixes: Ticket10227 > > Signed-off-by: Michael Niedermayer > --- > libavcodec/cscd.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Does the universe only have a finite lifespan? No, its going to go on forever, its just that you wont like living in it. -- Hiranya Peiri signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] A build error occurs when adding vaporsynth to ffmpeg.
There is a problem with commit 0c6e5f3. Missing changes in libavformat/vapoursynth.c. ___ 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/vapoursynth, avdevice/decklink_enc: Add missing includes
Broken in 0c6e5f321bf5c4054e8b98232692465b342b42b4.
Signed-off-by: Andreas Rheinhardt
---
libavdevice/decklink_enc.cpp | 1 +
libavformat/vapoursynth.c| 1 +
2 files changed, 2 insertions(+)
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index ffd0ad9250..cb8f91730e 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -33,6 +33,7 @@ extern "C" {
extern "C" {
#include "libavformat/avformat.h"
#include "libavcodec/bytestream.h"
+#include "libavutil/frame.h"
#include "libavutil/internal.h"
#include "libavutil/imgutils.h"
#include "avdevice.h"
diff --git a/libavformat/vapoursynth.c b/libavformat/vapoursynth.c
index 1578a6ac77..965d36dc2e 100644
--- a/libavformat/vapoursynth.c
+++ b/libavformat/vapoursynth.c
@@ -31,6 +31,7 @@
#include "libavutil/avassert.h"
#include "libavutil/avstring.h"
#include "libavutil/eval.h"
+#include "libavutil/frame.h"
#include "libavutil/imgutils.h"
#include "libavutil/opt.h"
#include "libavutil/pixdesc.h"
--
2.34.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] A build error occurs when adding vaporsynth to ffmpeg.
あんこ: > There is a problem with commit 0c6e5f3. Missing changes in > libavformat/vapoursynth.c. Thanks for the info (and sorry for the inconvenience). Does http://ffmpeg.org/pipermail/ffmpeg-devel/2023-September/314262.html fix it? - 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".
[FFmpeg-devel] [PATCH] hevc_ps: fix cpb_cnt_minus1 initialization and fixed_rate check
Fixes: fc429d785e9e24c5520ce716d4bc3b5547e581eb
cpb_cnt was initialized to 1 before
fc429d785e9e24c5520ce716d4bc3b5547e581eb, so cpb_cnt_minus1 should be
initialized to 0 instead of 1.
Since we split fixed_rate into a general flag and a within_cvs_flag now,
check for both in conditional.
---
libavcodec/hevc_ps.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index bdd623507d..da5ba5a3bf 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -406,12 +406,11 @@ static int decode_hrd(GetBitContext *gb, int
common_inf_present,
for (int i = 0; i < max_sublayers; i++) {
hdr->flags.fixed_pic_rate_general_flag = get_bits1(gb);
-hdr->cpb_cnt_minus1[i] = 1;
-
if (!hdr->flags.fixed_pic_rate_general_flag)
hdr->flags.fixed_pic_rate_within_cvs_flag = get_bits1(gb);
-if (hdr->flags.fixed_pic_rate_within_cvs_flag)
+if (hdr->flags.fixed_pic_rate_within_cvs_flag ||
+hdr->flags.fixed_pic_rate_general_flag)
hdr->elemental_duration_in_tc_minus1[i] = get_ue_golomb_long(gb);
else
hdr->flags.low_delay_hrd_flag = get_bits1(gb);
--
2.42.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] hevc_ps: fix cpb_cnt_minus1 initialization and fixed_rate check
On 9/10/2023 4:00 PM, llyyr wrote: > Fixes: fc429d785e9e24c5520ce716d4bc3b5547e581eb > > cpb_cnt was initialized to 1 before > fc429d785e9e24c5520ce716d4bc3b5547e581eb, so cpb_cnt_minus1 should be > initialized to 0 instead of 1. > > Since we split fixed_rate into a general flag and a within_cvs_flag now, > check for both in conditional. > --- > libavcodec/hevc_ps.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Should be two commits, I think. Also, cpb_cnt_minus1 is being passed to decode_sublayer_hrd() below, and needs a +1 there. - Derek ___ 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] A build error occurs when adding vaporsynth to ffmpeg.
Thank you for quick response! I can now build successfully. 2023年9月10日(日) 23:50 Andreas Rheinhardt : > あんこ: > > There is a problem with commit 0c6e5f3. Missing changes in > > libavformat/vapoursynth.c. > > Thanks for the info (and sorry for the inconvenience). Does > http://ffmpeg.org/pipermail/ffmpeg-devel/2023-September/314262.html fix > it? > > - 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". > ___ 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/2] hevc_ps: fix cpb_cnt_minus1 initialization
Fixes: fc429d785e9e24c5520ce716d4bc3b5547e581eb
cpb_cnt used to be initialized to 1 before
fc429d785e9e24c5520ce716d4bc3b5547e581eb so cpb_cnt_minus1 should be
initialized to 0.
Also add +1 to the decode_sublayer_hrd call to account for the change to
the offset
---
libavcodec/hevc_ps.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c
index bdd623507d..ac3fe55b07 100644
--- a/libavcodec/hevc_ps.c
+++ b/libavcodec/hevc_ps.c
@@ -406,8 +406,6 @@ static int decode_hrd(GetBitContext *gb, int
common_inf_present,
for (int i = 0; i < max_sublayers; i++) {
hdr->flags.fixed_pic_rate_general_flag = get_bits1(gb);
-hdr->cpb_cnt_minus1[i] = 1;
-
if (!hdr->flags.fixed_pic_rate_general_flag)
hdr->flags.fixed_pic_rate_within_cvs_flag = get_bits1(gb);
@@ -426,11 +424,11 @@ static int decode_hrd(GetBitContext *gb, int
common_inf_present,
}
if (hdr->flags.nal_hrd_parameters_present_flag)
-decode_sublayer_hrd(gb, hdr->cpb_cnt_minus1[i],
&hdr->nal_params[i],
+decode_sublayer_hrd(gb, hdr->cpb_cnt_minus1[i]+1,
&hdr->nal_params[i],
hdr->flags.sub_pic_hrd_params_present_flag);
if (hdr->flags.vcl_hrd_parameters_present_flag)
-decode_sublayer_hrd(gb, hdr->cpb_cnt_minus1[i],
&hdr->vcl_params[i],
+decode_sublayer_hrd(gb, hdr->cpb_cnt_minus1[i]+1,
&hdr->vcl_params[i],
hdr->flags.sub_pic_hrd_params_present_flag);
}
--
2.42.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 v2 2/2] hevc_ps: fix fixed_rate check
Fixes: fc429d785e9e24c5520ce716d4bc3b5547e581eb Since fc429d785e9e24c5520ce716d4bc3b5547e581eb splits the fixed_rate flag into general and within_cvs, check for both. --- libavcodec/hevc_ps.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c index ac3fe55b07..7507d2bf9c 100644 --- a/libavcodec/hevc_ps.c +++ b/libavcodec/hevc_ps.c @@ -409,7 +409,8 @@ static int decode_hrd(GetBitContext *gb, int common_inf_present, if (!hdr->flags.fixed_pic_rate_general_flag) hdr->flags.fixed_pic_rate_within_cvs_flag = get_bits1(gb); -if (hdr->flags.fixed_pic_rate_within_cvs_flag) +if (hdr->flags.fixed_pic_rate_within_cvs_flag || +hdr->flags.fixed_pic_rate_general_flag) hdr->elemental_duration_in_tc_minus1[i] = get_ue_golomb_long(gb); else hdr->flags.low_delay_hrd_flag = get_bits1(gb); -- 2.42.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 3/7] proresdec2: use VLC for level instead of EC switch
Hello, Le ven. 8 sept. 2023 à 11:57, Andreas Rheinhardt a écrit : > >> +#define CACHED_BITSTREAM_READER 1 > > > > This should be in the commit switching to the cached bitstream reader. > > Correction: This header is included in videotoolbox.c and there is other > stuff that also includes get_bits.h included in said file (and currently > gets included before proresdec.h). This means that proresdec2.c and > videotoolbox.c will have different opinions on what a GetBitContext is: > It will be the non-cached one in videotoolbox.c and the cached one in > proresdec2.c. This will work in practice, because ProresContext does not > need the complete GetBitContext type at all (it does not contain a > GetBitContext at all), so offsets are not affected. But it is > nevertheless undefined behaviour and could become dangerous when using LTO. > > So you should switch the type of the pointer to BitstreamContextBE* in > proresdec2.h. Furthermore, you can either include bitstream.h in > proresdec.h or (IMO better) use a forward declaration and struct > BitstreamContextBE* in the function pointer without including get_bits.h > in the header at all. On that point only: I don't recall (yes that's 3+ years old) the issue being videotoolbox, it didn't have that include back when I wrote this code. It's a very faint recollection, and I don't find proof in the ffmpeg code today or of 3+ years ago, but the problem you mention was happening with the encoder instead. So maybe the fix now is needed only by videotoolbox then. -- Christophe ___ 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 2/2] hevc_ps: fix fixed_rate check
Sep 10, 2023, 17:26 by llyyr.pub...@gmail.com: > Fixes: fc429d785e9e24c5520ce716d4bc3b5547e581eb > > Since fc429d785e9e24c5520ce716d4bc3b5547e581eb splits the fixed_rate > flag into general and within_cvs, check for both. > --- > libavcodec/hevc_ps.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/hevc_ps.c b/libavcodec/hevc_ps.c > index ac3fe55b07..7507d2bf9c 100644 > --- a/libavcodec/hevc_ps.c > +++ b/libavcodec/hevc_ps.c > @@ -409,7 +409,8 @@ static int decode_hrd(GetBitContext *gb, int > common_inf_present, > if (!hdr->flags.fixed_pic_rate_general_flag) > hdr->flags.fixed_pic_rate_within_cvs_flag = get_bits1(gb); > > -if (hdr->flags.fixed_pic_rate_within_cvs_flag) > +if (hdr->flags.fixed_pic_rate_within_cvs_flag || > +hdr->flags.fixed_pic_rate_general_flag) > hdr->elemental_duration_in_tc_minus1[i] = get_ue_golomb_long(gb); > else > hdr->flags.low_delay_hrd_flag = get_bits1(gb); > Both patches lgtm Thanks ___ 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 3/7] proresdec2: use VLC for level instead of EC switch
Christophe Gisquet: > Hello, > > Le ven. 8 sept. 2023 à 11:57, Andreas Rheinhardt > a écrit : +#define CACHED_BITSTREAM_READER 1 >>> >>> This should be in the commit switching to the cached bitstream reader. >> >> Correction: This header is included in videotoolbox.c and there is other >> stuff that also includes get_bits.h included in said file (and currently >> gets included before proresdec.h). This means that proresdec2.c and >> videotoolbox.c will have different opinions on what a GetBitContext is: >> It will be the non-cached one in videotoolbox.c and the cached one in >> proresdec2.c. This will work in practice, because ProresContext does not >> need the complete GetBitContext type at all (it does not contain a >> GetBitContext at all), so offsets are not affected. But it is >> nevertheless undefined behaviour and could become dangerous when using LTO. >> >> So you should switch the type of the pointer to BitstreamContextBE* in >> proresdec2.h. Furthermore, you can either include bitstream.h in >> proresdec.h or (IMO better) use a forward declaration and struct >> BitstreamContextBE* in the function pointer without including get_bits.h >> in the header at all. > > On that point only: I don't recall (yes that's 3+ years old) the issue > being videotoolbox, it didn't have that include back when I wrote this > code. > > It's a very faint recollection, and I don't find proof in the ffmpeg > code today or of 3+ years ago, but the problem you mention was > happening with the encoder instead. So maybe the fix now is needed > only by videotoolbox then. > The videotoolbox prores hwaccel has only been added in November 2021; before that, nobody but proresdec2.c included proresdec.h, so that there was no issue with GetBitContext being cached or not in different files. Another solution would be to use void* instead of GetBitContext* in the header and in the implementation and then convert this void* to GetBitContext* in the function. I do not know what you mean by "the encoder instead". What problem happens with the encoder? Why would the encoder include proresdec.h at all and why would it be affected by changes to the decoder? - 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 3/7] proresdec2: use VLC for level instead of EC switch
Hello Le dim. 10 sept. 2023 à 17:40, Andreas Rheinhardt a écrit : > Another solution would be to use void* instead of GetBitContext* in the > header and in the implementation and then convert this void* to > GetBitContext* in the function. The forward declaration will be enough. > I do not know what you mean by "the encoder instead". What problem > happens with the encoder? Why would the encoder include proresdec.h at > all and why would it be affected by changes to the decoder? The keyword was "faint", and it's a non-issue now. Just explaining why I would have had that code change that now appears as an attempted fix for videotoolbox, but really was never meant that way: pure luck, the actual reason being lost to time. -- Christophe ___ 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 01/21] avformat/avio: Don't use incompatible function pointer type for call
On Sun, 10 Sep 2023, Andreas Rheinhardt wrote: Marton Balint: On Sat, 9 Sep 2023, Tomas Härdin wrote: fre 2023-09-08 klockan 22:38 +0200 skrev Marton Balint: On Thu, 7 Sep 2023, Andreas Rheinhardt wrote: > It is undefined behaviour even in cases where it works > (it works because it is only a const uint8_t* vs. uint8_t* > difference). > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/avio.c | 25 - > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/libavformat/avio.c b/libavformat/avio.c > index ab1c19a58d..d53da5cb0c 100644 > --- a/libavformat/avio.c > +++ b/libavformat/avio.c > @@ -354,10 +354,15 @@ fail: > } > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t > *buf, > + const uint8_t *cbuf, > int size, int size_min, > - int > (*transfer_func)(URLContext *h, > - > uint8_t *buf, > - int > size)) > + int > (*read_func)(URLContext *h, > + uint8_t > *buf, > + int > size), > + int > (*write_func)(URLContext *h, > + const > uint8_t *buf, > + int > size), > + int read) These extra parameters are very ugly, can't we think of another way to properly support this? One idea is putting retry_transfer_wrapper in a template file and include it twice with proper defines-s for the read and write flavours. Seems like a lot of work for a function that's internal to avio.c If future extensibility is not important here then function pointers should not be passed to retry_tranfer_wrapper because h->prot->url_read/write can be used directly. And usage of buf/cbuf is readundant with the read paramter, because by checking if buf or cbuf is null you can decide the operation (read of write). The compiler does not know whether buf given to ffurl_(read|write|read_complete) is NULL or not in the first place (it also does not know whether the url_read and url_write function pointers are NULL or not); therefore if one use e.g. cbuf != NULL as meaning read == 0, then the write function would actually check for whether cbuf is NULL which is worse than it is now. (My initial version (not sent to this list) checked for whether the read function was NULL in order to determine whether we are reading or writing; the assumption was that the compiler would optimize the check away when reading, because if the read function were NULL, then a NULL function pointer would be used for a call, which is undefined behaviour. But it didn't. Instead it added ffurl_read.cold and ffurl_read_complete.cold functions (which presumably abort or so) for this case.) Maybe this could work to make the compiler optimize away the undeeded one: if (buf && !cbuf) write(); if (!buf && cbuf) read(); But v2 is also fine, use whichever you prefer. Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/21] avformat/avio: Don't use incompatible function pointer type for call
Marton Balint: > > > On Sun, 10 Sep 2023, Andreas Rheinhardt wrote: > >> Marton Balint: >>> >>> >>> On Sat, 9 Sep 2023, Tomas Härdin wrote: >>> fre 2023-09-08 klockan 22:38 +0200 skrev Marton Balint: > > > On Thu, 7 Sep 2023, Andreas Rheinhardt wrote: > > > It is undefined behaviour even in cases where it works > > (it works because it is only a const uint8_t* vs. uint8_t* > > difference). > > > Signed-off-by: Andreas Rheinhardt > > --- > > libavformat/avio.c | 25 - > > 1 file changed, 16 insertions(+), 9 deletions(-) > > > diff --git a/libavformat/avio.c b/libavformat/avio.c > > index ab1c19a58d..d53da5cb0c 100644 > > --- a/libavformat/avio.c > > +++ b/libavformat/avio.c > > @@ -354,10 +354,15 @@ fail: > > } > > > static inline int retry_transfer_wrapper(URLContext *h, uint8_t > > *buf, > > + const uint8_t *cbuf, > > int size, int size_min, > > - int > > (*transfer_func)(URLContext *h, > > - > > uint8_t *buf, > > - int > > size)) > > + int > > (*read_func)(URLContext *h, > > + uint8_t > > *buf, > > + int > > size), > > + int > > (*write_func)(URLContext *h, > > + const > > uint8_t *buf, > > + int > > size), > > + int read) > > These extra parameters are very ugly, can't we think of another way > to properly support this? > > One idea is putting retry_transfer_wrapper in a template file and > include it twice with proper defines-s for the read and write > flavours. Seems like a lot of work for a function that's internal to avio.c >>> >>> If future extensibility is not important here then function pointers >>> should not be passed to retry_tranfer_wrapper because >>> h->prot->url_read/write can be used directly. And usage of buf/cbuf is >>> readundant with the read paramter, because by checking if buf or cbuf is >>> null you can decide the operation (read of write). >>> >> >> The compiler does not know whether buf given to >> ffurl_(read|write|read_complete) is NULL or not in the first place (it >> also does not know whether the url_read and url_write function pointers >> are NULL or not); therefore if one use e.g. cbuf != NULL as meaning read >> == 0, then the write function would actually check for whether cbuf is >> NULL which is worse than it is now. >> (My initial version (not sent to this list) checked for whether the read >> function was NULL in order to determine whether we are reading or >> writing; the assumption was that the compiler would optimize the check >> away when reading, because if the read function were NULL, then a NULL >> function pointer would be used for a call, which is undefined behaviour. >> But it didn't. Instead it added ffurl_read.cold and >> ffurl_read_complete.cold functions (which presumably abort or so) for >> this case.) > > Maybe this could work to make the compiler optimize away the undeeded one: > > if (buf && !cbuf) > write(); > if (!buf && cbuf) > read(); > I don't get how this would help (apart from the fact that you switched write and read): The compiler knows that cbuf is NULL for reading, which means it can optimize away the second if, but it doesn't know whether buf is NULL and therefore will add an explicit (and unnecessary) check in the first if. So this is worse code. And apart from that, such a version would also rely on passing buf and cbuf and (potentially) the callbacks; I consider this worse and more obscure than a simple read parameter. > But v2 is also fine, use whichever you prefer. > Will do as soon as Tomas ok's this version. - 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".
[FFmpeg-devel] [PATCH] avfilter/x86/af_afir: add FMA3 SIMD
Attached.
From 7735a84fd0fdae731955f50bddba8dfef395713b Mon Sep 17 00:00:00 2001
From: Paul B Mahol
Date: Sun, 10 Sep 2023 19:25:20 +0200
Subject: [PATCH] avfilter/x86/af_afir: add FMA3 SIMD
Signed-off-by: Paul B Mahol
---
libavfilter/x86/af_afir.asm| 27 +++
libavfilter/x86/af_afir_init.c | 5 +
2 files changed, 32 insertions(+)
diff --git a/libavfilter/x86/af_afir.asm b/libavfilter/x86/af_afir.asm
index 2cc09709a2..ed0276c7b9 100644
--- a/libavfilter/x86/af_afir.asm
+++ b/libavfilter/x86/af_afir.asm
@@ -67,3 +67,30 @@ INIT_XMM sse3
FCMUL_ADD
INIT_YMM avx
FCMUL_ADD
+
+%if HAVE_FMA3_EXTERNAL
+INIT_YMM fma3
+cglobal fcmul_add, 4,4,4, sum, t, c, len
+shl lend, 3
+add tq, lenq
+add cq, lenq
+add sumq, lenq
+neg lenq
+.loop:
+movapsm0, [tq + lenq]
+movapsm1, [cq + lenq]
+vpermilps m3, m0, 177
+vpermilps m2, m1, 160
+vpermilps m1, m1, 245
+mulps m1, m1, m3
+vfmaddsub132ps m0, m1, m2
+addps m0, m0, [sumq + lenq]
+movaps[sumq + lenq], m0
+add lenq, mmsize
+jl .loop
+movss xm0, [tq + lenq]
+mulss xm0, [cq + lenq]
+addss xm0, [sumq + lenq]
+movss [sumq + lenq], xm0
+RET
+%endif
diff --git a/libavfilter/x86/af_afir_init.c b/libavfilter/x86/af_afir_init.c
index e53817b9c0..d573acf10b 100644
--- a/libavfilter/x86/af_afir_init.c
+++ b/libavfilter/x86/af_afir_init.c
@@ -26,6 +26,8 @@ void ff_fcmul_add_sse3(float *sum, const float *t, const float *c,
ptrdiff_t len);
void ff_fcmul_add_avx(float *sum, const float *t, const float *c,
ptrdiff_t len);
+void ff_fcmul_add_fma3(float *sum, const float *t, const float *c,
+ ptrdiff_t len);
av_cold void ff_afir_init_x86(AudioFIRDSPContext *s)
{
@@ -37,4 +39,7 @@ av_cold void ff_afir_init_x86(AudioFIRDSPContext *s)
if (EXTERNAL_AVX_FAST(cpu_flags)) {
s->fcmul_add = ff_fcmul_add_avx;
}
+if (EXTERNAL_FMA3_FAST(cpu_flags)) {
+s->fcmul_add = ff_fcmul_add_fma3;
+}
}
--
2.39.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 2/2] hevc_ps: fix fixed_rate check
On 9/10/2023 4:32 PM, Lynne wrote: > Both patches lgtm Pushed. ___ 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] avcodec/8bps: switch to planar RGB formats
Attached.
From 8ee65119916a849a37b39b3a8c12ca8af3b456c5 Mon Sep 17 00:00:00 2001
From: Paul B Mahol
Date: Sun, 10 Sep 2023 22:42:11 +0200
Subject: [PATCH 2/2] avcodec/8bps: always decode to planar formats directly
Signed-off-by: Paul B Mahol
---
libavcodec/8bps.c | 60 ---
1 file changed, 25 insertions(+), 35 deletions(-)
diff --git a/libavcodec/8bps.c b/libavcodec/8bps.c
index 15c236f114..0becaa9320 100644
--- a/libavcodec/8bps.c
+++ b/libavcodec/8bps.c
@@ -26,22 +26,18 @@
* http://www.pcisys.net/~melanson/codecs/
*
* Supports: PAL8 (RGB 8bpp, paletted)
- * : BGR24 (RGB 24bpp) (can also output it as RGB32)
- * : RGB32 (RGB 32bpp, 4th plane is alpha)
+ * : GBRP (RGB 24bpp)
+ * : GBRAP (RGB 32bpp, 4th plane is alpha)
*/
#include
-#include "libavutil/bswap.h"
+#include "libavutil/intreadwrite.h"
#include "libavutil/internal.h"
#include "avcodec.h"
#include "codec_internal.h"
#include "decode.h"
-
-static const enum AVPixelFormat pixfmt_rgb24[] = {
-AV_PIX_FMT_BGR24, AV_PIX_FMT_0RGB32, AV_PIX_FMT_NONE };
-
typedef struct EightBpsContext {
AVCodecContext *avctx;
@@ -61,9 +57,8 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
unsigned int dlen, p, row;
const uint8_t *lp, *dp, *ep;
uint8_t count;
-unsigned int px_inc;
-unsigned int planes = c->planes;
-uint8_t *planemap = c->planemap;
+const uint8_t *planemap = c->planemap;
+unsigned int planes = c->planes;
int ret;
if (buf_size < planes * height * 2)
@@ -77,19 +72,18 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
/* Set data pointer after line lengths */
dp = encoded + planes * (height << 1);
-px_inc = planes + (avctx->pix_fmt == AV_PIX_FMT_0RGB32);
-
for (p = 0; p < planes; p++) {
+const int pi = planemap[p];
/* Lines length pointer for this plane */
lp = encoded + p * (height << 1);
/* Decode a plane */
for (row = 0; row < height; row++) {
-pixptr = frame->data[0] + row * frame->linesize[0] + planemap[p];
-pixptr_end = pixptr + frame->linesize[0];
+pixptr = frame->data[pi] + row * frame->linesize[pi];
+pixptr_end = pixptr + frame->linesize[pi];
if (ep - lp < row * 2 + 2)
return AVERROR_INVALIDDATA;
-dlen = av_be2ne16(*(const uint16_t *)(lp + row * 2));
+dlen = AV_RB16(lp + row * 2);
/* Decode a row of this plane */
while (dlen > 0) {
if (ep - dp <= 1)
@@ -97,22 +91,19 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame,
if ((count = *dp++) <= 127) {
count++;
dlen -= count + 1;
-if (pixptr_end - pixptr < count * px_inc)
+if (pixptr_end - pixptr < count)
break;
if (ep - dp < count)
return AVERROR_INVALIDDATA;
-while (count--) {
-*pixptr = *dp++;
-pixptr += px_inc;
-}
+memcpy(pixptr, dp, count);
+pixptr += count;
+dp += count;
} else {
count = 257 - count;
-if (pixptr_end - pixptr < count * px_inc)
+if (pixptr_end - pixptr < count)
break;
-while (count--) {
-*pixptr = *dp;
-pixptr += px_inc;
-}
+memset(pixptr, dp[0], count);
+pixptr += count;
dp++;
dlen -= 2;
}
@@ -150,16 +141,15 @@ static av_cold int decode_init(AVCodecContext *avctx)
c->planemap[0] = 0; // 1st plane is palette indexes
break;
case 24:
-avctx->pix_fmt = ff_get_format(avctx, pixfmt_rgb24);
+avctx->pix_fmt = AV_PIX_FMT_GBRP;
c->planes = 3;
c->planemap[0] = 2; // 1st plane is red
-c->planemap[1] = 1; // 2nd plane is green
-c->planemap[2] = 0; // 3rd plane is blue
+c->planemap[1] = 0; // 2nd plane is green
+c->planemap[2] = 1; // 3rd plane is blue
break;
case 32:
-avctx->pix_fmt = AV_PIX_FMT_RGB32;
+avctx->pix_fmt = AV_PIX_FMT_GBRAP;
c->planes = 4;
-/* handle planemap setup later for decoding rgb24 data as rbg32 */
break;
default:
av_log(avctx, AV_LOG_ERROR, "Error: Unsupported color depth: %u.\n",
@@ -167,11 +157,11 @@ static av_cold int decode_init(AVCodecContext *avctx)
return AVERROR_INVALIDDATA;
}
-if (avctx->pix_fmt == AV_PIX_FMT_RGB32) {
-c->planemap[0] = HAVE_BIGENDIAN ? 1 : 2; // 1s
Re: [FFmpeg-devel] Trac spam
On Sun, Sep 10, 2023 at 11:20:23AM +0200, Michael Koch wrote: > new spammer in ticket 2104 i added you to the spam fighters group that gives you the following powers: SPAM_CHECKREPORTS SPAM_MONITOR SPAM_REPORT SPAM_TRAIN SPAM_USER TICKET_EDIT_COMMENT WIKI_ADMIN permissions sadly are not as fine grained as i would like, iam not sure these are enough to effectively remove spam but maybe some are still useful also the following may or may not work: 1. To monitor submissions to trac trac ffmpeg org/admin/spamfilter/monitor (each report should have a "Delete as spam" and "Remove registered user" button Note the "Delete as spam" ONLY seems to delete the report and probably submit to spam tracking services like akismet and update bayesian not delete the user nor her comments so you want to delete the user before the report 2. To see all changes a user has done: trac ffmpeg org/admin/spamfilter/user?mode=user&user=EVILSPAMMER 3. To see user submitted spam reports trac ffmpeg org/admin/spamfilter/report thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Take away the freedom of one citizen and you will be jailed, take away the freedom of all citizens and you will be congratulated by your peers in Parliament. 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 v27 2/2] avcodec/evc_decoder: Provided support for EVC decoder
On 8/16/2023 8:11 AM, Dawid Kozinski wrote:
+/**
+ * Initialize decoder
+ * Create a decoder instance and allocate all the needed resources
+ *
+ * @param avctx codec context
+ * @return 0 on success, negative error code on failure
+ */
+static av_cold int libxevd_init(AVCodecContext *avctx)
+{
+XevdContext *xectx = avctx->priv_data;
+XEVD_CDSC *cdsc = &(xectx->cdsc);
+
+/* read configurations and set values for created descriptor (XEVD_CDSC) */
+get_conf(avctx, cdsc);
+
+/* create decoder */
+xectx->id = xevd_create(&(xectx->cdsc), NULL);
+if (xectx->id == NULL) {
+av_log(avctx, AV_LOG_ERROR, "Cannot create XEVD encoder\n");
+return AVERROR_EXTERNAL;
+}
+
+xectx->draining_mode = 0;
+xectx->pkt = av_packet_alloc();
Unchecked allocation.
+
+return 0;
+}
+
+/**
+ * Decode frame with decoupled packet/frame dataflow
+ *
+ * @param avctx codec context
+ * @param[out] frame decoded frame
+ *
+ * @return 0 on success, negative error code on failure
+ */
+static int libxevd_receive_frame(AVCodecContext *avctx, AVFrame *frame)
+{
+XevdContext *xectx = avctx->priv_data;
+AVPacket *pkt = xectx->pkt;
+XEVD_IMGB *imgb = NULL;
+
+int xevd_ret = 0;
+int ret = 0;
+
+if (!pkt)
+return AVERROR(ENOMEM);
This check should be in libxevd_init(), like i said above.
+
+// obtain access unit (input data) - a set of NAL units that are
consecutive in decoding order and containing exactly one encoded image
+ret = ff_decode_get_packet(avctx, pkt);
You're unconditionally fetching a new packet every time receive_frame()
is called. Is it guaranteed that the previous packet was fully consumed
and freed?
+if (ret < 0 && ret != AVERROR_EOF) {
+av_packet_unref(pkt);
+
+return ret;
+} else if(ret == AVERROR_EOF && xectx->draining_mode == 0) { // End of
stream situations. Enter draining mode
+
+xectx->draining_mode = 1;
+av_packet_unref(pkt);
+}
+
+if (pkt->size > 0) {
+int bs_read_pos = 0;
+XEVD_STAT stat;
+XEVD_BITB bitb;
+int nalu_size;
+AVPacket* pkt_au;
+imgb = NULL;
+
+pkt_au = av_packet_clone(pkt);
Unchecked allocation.
+av_packet_unref(pkt);
You're unreferencing this packet here but then you check its fields below.
+
+// get all nal units from AU
+while(pkt_au->size > (bs_read_pos + XEVD_NAL_UNIT_LENGTH_BYTE)) {
+memset(&stat, 0, sizeof(XEVD_STAT));
+
+nalu_size = read_nal_unit_length(pkt_au->data + bs_read_pos,
XEVD_NAL_UNIT_LENGTH_BYTE, avctx);
+if (nalu_size == 0) {
+av_log(avctx, AV_LOG_ERROR, "Invalid bitstream\n");
+av_packet_free(&pkt_au);
+ret = AVERROR_INVALIDDATA;
+
+return ret;
+}
+bs_read_pos += XEVD_NAL_UNIT_LENGTH_BYTE;
+
+bitb.addr = pkt_au->data + bs_read_pos;
+bitb.ssize = nalu_size;
+bitb.pdata[0] = pkt_au;
+bitb.ts[XEVD_TS_DTS] = pkt_au->dts;
+
+/* main decoding block */
+xevd_ret = xevd_decode(xectx->id, &bitb, &stat);
+if (XEVD_FAILED(xevd_ret)) {
+av_log(avctx, AV_LOG_ERROR, "Failed to decode bitstream\n");
+av_packet_free(&pkt_au);
+ret = AVERROR_EXTERNAL;
You can just do return AVERROR_EXTERNAL;
+
+return ret;
+}
+
+bs_read_pos += nalu_size;
+
+if (stat.nalu_type == XEVD_NUT_SPS) { // EVC stream parameters
changed
+if ((ret = export_stream_params(xectx, avctx)) != 0) {
+av_log(avctx, AV_LOG_ERROR, "Failed to export stream
params\n");
+av_packet_free(&pkt_au);
+
+return ret;
+}
+}
+
+if (stat.read != nalu_size)
+av_log(avctx, AV_LOG_INFO, "Different reading of bitstream
(in:%d,read:%d)\n,", nalu_size, stat.read);
+
+// stat.fnum - has negative value if the decoded data is not frame
+if (stat.fnum >= 0) {
This means there can be more than one frame after a call to
xevd_decode() with one AU, right? Shouldn't you call xevd_pull() in a
loop before you call xevd_decode() again, or attempt to fetch another
packet/AU?
+
+xevd_ret = xevd_pull(xectx->id, &imgb); // The function
returns a valid image only if the return code is XEVD_OK
+
+if (XEVD_FAILED(xevd_ret)) {
+av_log(avctx, AV_LOG_ERROR, "Failed to pull the decoded image
(xevd error code: %d, frame#=%d)\n", xevd_ret, stat.fnum);
+ret = AVERROR_EXTERNAL;
+av_packet_free(&pkt_au);
+
+return ret;
+} else if (xevd_ret == XEVD_OK_FRM_DELAYED) {
+if(bs_read_pos == pkt->size) {
This is the check
