[FFmpeg-devel] PATCH: Fallback to NV12 format in VAAPI drivers
On Fri, Aug 09, 2024 at 09:43:53AM +0200, Lluís Batlle i Rossell wrote: > On Fri, Aug 09, 2024 at 11:49:54AM +0800, Zhao Zhili wrote: > > > vaapi drivers often lack proper image converesions and not all > > > situations allow vagetimage or vaputimage with the image formats > > > reported by the api. nv12 seems allowed in all circumstances. > > > > > > with this change now one can use the hwaccel directly without > > > explicit conversions to nv12 for frame downloading to work. > > > > > > gstreamer adopted a similar approach: > > > https://bugzilla.gnome.org/show_bug.cgi?id=752958 > > > > Isn’t it break all pixel formats with bit depth > 8? > > I think we already have hwcontext API to select sw_format, this isn’t a bug > > inside ffmpeg. > > Correct... I didn't think of the need beyond NV12. > > What if I redo the patch so I keep all formats, but I simply move NV12 to > the first place? That will make ffmpeg pick NV12 as default if NONE > specified. I attach a different patch, so NV12 is only picked in case the dst format is NONE. >From c633b6ba0975bd495df2479aa9a562958bb87e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Llu=C3=ADs=20Batlle=20i=20Rossell?= Date: Sat, 10 Aug 2024 10:45:14 +0200 Subject: [PATCH] Fallback to NV12 format in VAAPI drivers Even if the hw format is listed in the supported ImageFormats. That's because some drivers fail in vaGetImage even if requesting the transfer in a format equal to the hw format. NV12 is chosen only if the users don't specify any other particular format. gstreamer adopted a similar approach: https://bugzilla.gnome.org/show_bug.cgi?id=752958 --- libavutil/hwcontext_vaapi.c | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c index 12bc95119a..0396038017 100644 --- a/libavutil/hwcontext_vaapi.c +++ b/libavutil/hwcontext_vaapi.c @@ -720,20 +720,22 @@ static int vaapi_transfer_get_formats(AVHWFramesContext *hwfc, { VAAPIDeviceContext *ctx = hwfc->device_ctx->internal->priv; enum AVPixelFormat *pix_fmts; -int i, k, sw_format_available; +int i, k, nv12_format_available; -sw_format_available = 0; +/* Intel VAAPI drivers seem to support NV12 for vaGetImage, + * but fail for many formats announced in vaQueryImageFormats */ +nv12_format_available = 0; for (i = 0; i < ctx->nb_formats; i++) { -if (ctx->formats[i].pix_fmt == hwfc->sw_format) -sw_format_available = 1; +if (ctx->formats[i].pix_fmt == AV_PIX_FMT_NV12) +nv12_format_available = 1; } pix_fmts = av_malloc((ctx->nb_formats + 1) * sizeof(*pix_fmts)); if (!pix_fmts) return AVERROR(ENOMEM); -if (sw_format_available) { -pix_fmts[0] = hwfc->sw_format; +if (nv12_format_available) { +pix_fmts[0] = AV_PIX_FMT_NV12; k = 1; } else { k = 0; -- 2.44.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/vpk: fix divide by zero
Kacper Michajlow: > On Fri, 9 Aug 2024 at 22:51, Michael Niedermayer > wrote: >> >> On Wed, Aug 07, 2024 at 03:42:46PM +0200, Kacper Michajłow wrote: >>> Can happen after calling avformat_find_stream_info() when the codec >>> fails to open, but return value is 0 and subsequent uses of this context >>> have zero value in channel number. >>> >>> Found by OSS-Fuzz. >>> >>> Signed-off-by: Kacper Michajłow >>> --- >>> libavformat/vpk.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavformat/vpk.c b/libavformat/vpk.c >>> index 001ad33555..aa98ef2dd4 100644 >>> --- a/libavformat/vpk.c >>> +++ b/libavformat/vpk.c >>> @@ -86,6 +86,8 @@ static int vpk_read_packet(AVFormatContext *s, AVPacket >>> *pkt) >>> >>> vpk->current_block++; >>> if (vpk->current_block == vpk->block_count) { >>> +if (par->ch_layout.nb_channels <= 0) >>> +return AVERROR_INVALIDDATA; >>> unsigned size = vpk->last_block_size / par->ch_layout.nb_channels; >>> unsigned skip = (par->block_align - vpk->last_block_size) / >>> par->ch_layout.nb_channels; >>> uint64_t pos = avio_tell(s->pb); >> >> iam not sure if a parser or other should replace a valid set of >> parameters by an invalid >> (this patch implies that such a action occured) >> >> can you explain more detailedly by what and why channels is set to 0 ? >> > > You are right, it might be better to improve this to not override the > params. Let me explain what happens, I didn't read through the whole > avformat_find_stream_info() to know what would be the best approach > yet. I will try to look at it, but if you have immediate ideas, that > would be nice. > > 1. avformat_open_input() sets nb_channels to 108 > > 2. Just after that we call avformat_find_stream_info(avfc, NULL); this > returns 0 (success), but as a result it overrides params already > present in the context. > log for reference, during the find stream info call > [ffmpeg/demuxer] vpk: Before avformat_find_stream_info() pos: > 538976288 bytes read:21 seeks:1 nb_streams:1 > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > [lavf] mp_seek(0x51218090, 0, size) > [lavf] 0=mp_read(0x51218090, 0x7fe4c7ce8800, 5000), pos: > 538976288, eof:1 > [lavf] 0=mp_read(0x51218090, 0x52d0a400, 32768), pos: 538976288, eof:1 > [ffmpeg/audio] adpcm_psx: Decoder requires channel layout to be set > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > [lavf] mp_seek(0x51218090, 0, size) > [lavf] mp_seek(0x51218090, 0, size) > [lavf] mp_seek(0x51218090, 0, size) > [ffmpeg/demuxer] vpk: stream 0: start_time: NOPTS duration: 0.069852 > [ffmpeg/demuxer] vpk: format: start_time: NOPTS duration: 0.069852 > (estimate from stream) bitrate=2 kb/s > [ffmpeg/demuxer] vpk: Could not find codec parameters for stream 0 > (Audio: adpcm_psx, 538976288 Hz, 0 channels): unspecified sample > format > [ffmpeg/demuxer] Consider increasing the value for the > 'analyzeduration' (0) and 'probesize' (500) options > [ffmpeg/demuxer] vpk: After avformat_find_stream_info() pos: 538976288 > bytes read:21 seeks:1 frames:0 > > 3. the nb_channels value is cleared in avformat_find_stream_info() -> > avcodec_parameters_from_context() -> codec_parameters_reset() and > remains 0. This seems like the error: Why is AVCodecParameters being set from an AVCodecContext if the codec could not be successfully opened? > > 4. as we can see there were errors, but it still returns success, so we > proceed. > > 5. on the next av_seek_frame() which goes to vpk_read_packet() it will > fail because nb_channels is 0 at this point. > > Sorry for only a high level overview, but at this moment, I'm not sure > how exactly it is supposed to work. I thought it might be intended to > override headers parameters later if we know better later on, that's > why my initial patch only tackled this case. > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_videotoolbox: silence warning for RGB
On 10 Aug 2024, at 8:13, gnattu via ffmpeg-devel wrote: > Hardware frames with RGB colorspace will not have a YCbCrMatrixKey. > Currently, it will spam the console with warning if rgb frame is > uploaded. Thanks, LGTM > > Signed-off-by: Gnattu OC > --- > libavutil/hwcontext_videotoolbox.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavutil/hwcontext_videotoolbox.c > b/libavutil/hwcontext_videotoolbox.c > index 80eaab64f08..122a61d5e78 100644 > --- a/libavutil/hwcontext_videotoolbox.c > +++ b/libavutil/hwcontext_videotoolbox.c > @@ -576,7 +576,7 @@ static int vt_pixbuf_set_colorspace(void *log_ctx, > colormatrix, kCVAttachmentMode_ShouldPropagate); > else { > CVBufferRemoveAttachment(pixbuf, kCVImageBufferYCbCrMatrixKey); > -if (src->colorspace != AVCOL_SPC_UNSPECIFIED) > +if (src->colorspace != AVCOL_SPC_UNSPECIFIED && src->colorspace != > AVCOL_SPC_RGB) > av_log(log_ctx, AV_LOG_WARNING, > "Color space %s is not supported.\n", > av_color_space_name(src->colorspace)); > -- > 2.39.3 (Apple Git-146) > > ___ > 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 5/6] tools/target_dec_fuzzer: Use av_buffer_allocz() to avoid missing slices to have unpredictable content
On 8/9/2024 5:09 PM, Michael Niedermayer wrote: Hi On Fri, Aug 09, 2024 at 03:56:42AM +0200, Kacper Michajlow wrote: On Fri, 9 Aug 2024 at 00:06, Michael Niedermayer wrote: On Thu, Aug 08, 2024 at 02:13:12PM -0300, James Almer wrote: [...] If decoders are fed with uninitialized buffers thats a security issue because there are thousands if not ten thousands of pathes if you consider the number of decoders and the number of ways they can hit errors Clearing those buffers in fuzzers does not alleviate this security issue, as they may still be uninitialized in production code. The decoders in production clear the buffers. The fuzzer does not so the issues it shows dont exist in production look yourself in get_buffer.c pool->pools[i] = av_buffer_pool_init(size[i] + 16 + STRIDE_ALIGN - 1, CONFIG_MEMORY_POISONING ? NULL : av_buffer_allocz); its av_buffer_allocz I disagree. That's from avcodec_default_get_buffer2(). What about DR1 decoders where the caller is using their own avctx.get_buffer2() callback? Nothing in the documentation says that the buffers must be zeroed. I wrote the function you just changed with the intention of finding issues a library user could trigger, which included allocating buffers exactly as big as needed (with no extra padding) and not zeroing it, using lavu helpers like the get_buffer2() documentation states. This change here makes half of that moot, and is hiding potential bugs in the form of use of uninitialized memory in our decoders. [...] Security wise this is not possible for production code, its too fragile (at least with the number of decoders and active maintainers we have) (you want less code to have to be bugfree for security not more code having to be bug free) Now this is the fuzzer and not production code, ok. And of course is great to have error concealment in every decoder But then this leaves the question, who will do this work? If noone does it then we will accumulate many msan bugs in ossfuzz that we wont be able to do much with except ignore them. This would make the fuzzer less efficient and it would confuse people looking at the issues MSAN is not forgiving, and I can imagine that stabilizing it could take time. However, suppressing the reports will not make it more efficient. It will make it more efficient because then the fuzzer shows only issues also affecting production and ones someone intends to work on Otherwise it shows many issues that will distract and confuse I might not fully understand what you meant, though. Yes, i think we misunderstand each other a bit [...] Perhaps it should be configurable per decoder. That is what i suggested, or at least i meant to. For decoders where someone intends to fix every case where original buffer data with nothing written into it come through it could make sense to enable uninitialized input buffers. Still i have not seen anyone actually want to do that. I certainly dont have the time for any of the decoders that i maintain. But if someone else wants i surely dont mind if (s)he turns this on and works on the additional cases for any decoders that i maintain ... Or the short punchy reply maybe is Produce a volunteer who will fix these bugs before declaring them bugs. And when doing so consider that we have bugfixes on the mailing list for which we seem to not even have the man power to review and apply them so yeah my oppinion is the default should be the simple & easy to maintain way. If someone declares their decoder to have flawless error concealment (and for some simple decoders that could be quite simple) these can always be excluded and use uninitialized buffers in the fuzzer What is the problem with keeping those reports and letting "someone" work on their decoder based on reports? ossfuzz is the problem, these issues are not seperate/segregated nor do i see a way ossfuzz could seperate them but again ATM we have noone intending to work on this so this patch solves it. thx [...] ___ 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] libavutil: deprecate the old Vulkan queue API, bump minor and add APIchanges
--- doc/APIchanges | 9 + libavutil/hwcontext_vulkan.c | 8 libavutil/hwcontext_vulkan.h | 12 libavutil/version.h | 3 ++- 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/doc/APIchanges b/doc/APIchanges index 046828ded1..173f317ea1 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -2,6 +2,15 @@ The last version increases of all libraries were on 2024-03-07 API changes, most recent first: +2024-08-10 - x - lavu 59.34.100 - hwcontext_vulkan.h + Add qf and nb_qf to AVVulkanDeviceContext. + Deprecate queue_family_index, nb_graphics_queues, + queue_family_tx_index, nb_tx_queues. + queue_family_comp_index, nb_comp_queues. + queue_family_encode_index, nb_encode_queues. + queue_family_decode_index, and nb_decode_queues, + from AVVulkanDeviceContext. + 2024-07-30 - x - lavu 59.32.100 - cpu.h Deprecate AV_CPU_FLAG_RVF and AV_CPU_FLAG_RVD without replacement. Deprecate AV_CPU_FLAG_RVB_ADDR, subsumed into AV_CPU_FLAG_RVB. diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index 05fadd1b55..59d519727b 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -1228,6 +1228,8 @@ static int setup_queue_families(AVHWDeviceContext *ctx, VkDeviceCreateInfo *cd) }; } +#if FF_API_VULKAN_FIXED_QUEUES +FF_DISABLE_DEPRECATION_WARNINGS /* Setup deprecated fields */ hwctx->queue_family_index= -1; hwctx->queue_family_comp_index = -1; @@ -1252,6 +1254,8 @@ static int setup_queue_families(AVHWDeviceContext *ctx, VkDeviceCreateInfo *cd) } #undef SET_OLD_QF +FF_ENABLE_DEPRECATION_WARNINGS +#endif return 0; } @@ -1611,6 +1615,8 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) } } +#if FF_API_VULKAN_FIXED_QUEUES +FF_DISABLE_DEPRECATION_WARNINGS graph_index = hwctx->nb_graphics_queues ? hwctx->queue_family_index : -1; comp_index = hwctx->nb_comp_queues ? hwctx->queue_family_comp_index : -1; tx_index= hwctx->nb_tx_queues ? hwctx->queue_family_tx_index : -1; @@ -1678,6 +1684,8 @@ static int vulkan_device_init(AVHWDeviceContext *ctx) ADD_QUEUE(hwctx->queue_family_encode_index, hwctx->nb_encode_queues, VK_QUEUE_VIDEO_ENCODE_BIT_KHR); #undef ADD_QUEUE } +FF_ENABLE_DEPRECATION_WARNINGS +#endif for (int i = 0; i < hwctx->nb_qf; i++) { if (!hwctx->qf[i].video_caps && diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h index e9e42015f7..7f56e96e6e 100644 --- a/libavutil/hwcontext_vulkan.h +++ b/libavutil/hwcontext_vulkan.h @@ -113,6 +113,7 @@ typedef struct AVVulkanDeviceContext { const char * const *enabled_dev_extensions; int nb_enabled_dev_extensions; +#if FF_API_VULKAN_FIXED_QUEUES /** * Queue family index for graphics operations, and the number of queues * enabled for it. If unavaiable, will be set to -1. Not required. @@ -120,21 +121,27 @@ typedef struct AVVulkanDeviceContext { * queue family, or pick the one with the least unrelated flags set. * Queue indices here may overlap if a queue has to share capabilities. */ +attribute_deprecated int queue_family_index; +attribute_deprecated int nb_graphics_queues; /** * Queue family index for transfer operations and the number of queues * enabled. Required. */ +attribute_deprecated int queue_family_tx_index; +attribute_deprecated int nb_tx_queues; /** * Queue family index for compute operations and the number of queues * enabled. Required. */ +attribute_deprecated int queue_family_comp_index; +attribute_deprecated int nb_comp_queues; /** @@ -142,7 +149,9 @@ typedef struct AVVulkanDeviceContext { * If the device doesn't support such, queue_family_encode_index will be -1. * Not required. */ +attribute_deprecated int queue_family_encode_index; +attribute_deprecated int nb_encode_queues; /** @@ -150,8 +159,11 @@ typedef struct AVVulkanDeviceContext { * If the device doesn't support such, queue_family_decode_index will be -1. * Not required. */ +attribute_deprecated int queue_family_decode_index; +attribute_deprecated int nb_decode_queues; +#endif /** * Locks a queue, preventing other threads from submitting any command diff --git a/libavutil/version.h b/libavutil/version.h index c8db361ddb..84eb3a388a 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 59 -#define LIBAVUTIL_VERSION_MINOR 33 +#define LIBAVUTIL_VERSION_MINOR 34 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ @@ -114,6 +114,7 @@ #define FF_API_H274_FILM_GRAIN_VCS (LIBAVUTIL_VERSION_MAJOR < 60) #define FF_API_MOD_UINTP2 (LIBAVUTIL_VERSIO
Re: [FFmpeg-devel] [PATCH] avformat/vpk: fix divide by zero
On Sat, 10 Aug 2024 at 11:25, Andreas Rheinhardt wrote: > > Kacper Michajlow: > > On Fri, 9 Aug 2024 at 22:51, Michael Niedermayer > > wrote: > >> > >> On Wed, Aug 07, 2024 at 03:42:46PM +0200, Kacper Michajłow wrote: > >>> Can happen after calling avformat_find_stream_info() when the codec > >>> fails to open, but return value is 0 and subsequent uses of this context > >>> have zero value in channel number. > >>> > >>> Found by OSS-Fuzz. > >>> > >>> Signed-off-by: Kacper Michajłow > >>> --- > >>> libavformat/vpk.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/libavformat/vpk.c b/libavformat/vpk.c > >>> index 001ad33555..aa98ef2dd4 100644 > >>> --- a/libavformat/vpk.c > >>> +++ b/libavformat/vpk.c > >>> @@ -86,6 +86,8 @@ static int vpk_read_packet(AVFormatContext *s, AVPacket > >>> *pkt) > >>> > >>> vpk->current_block++; > >>> if (vpk->current_block == vpk->block_count) { > >>> +if (par->ch_layout.nb_channels <= 0) > >>> +return AVERROR_INVALIDDATA; > >>> unsigned size = vpk->last_block_size / > >>> par->ch_layout.nb_channels; > >>> unsigned skip = (par->block_align - vpk->last_block_size) / > >>> par->ch_layout.nb_channels; > >>> uint64_t pos = avio_tell(s->pb); > >> > >> iam not sure if a parser or other should replace a valid set of > >> parameters by an invalid > >> (this patch implies that such a action occured) > >> > >> can you explain more detailedly by what and why channels is set to 0 ? > >> > > > > You are right, it might be better to improve this to not override the > > params. Let me explain what happens, I didn't read through the whole > > avformat_find_stream_info() to know what would be the best approach > > yet. I will try to look at it, but if you have immediate ideas, that > > would be nice. > > > > 1. avformat_open_input() sets nb_channels to 108 > > > > 2. Just after that we call avformat_find_stream_info(avfc, NULL); this > > returns 0 (success), but as a result it overrides params already > > present in the context. > > log for reference, during the find stream info call > > [ffmpeg/demuxer] vpk: Before avformat_find_stream_info() pos: > > 538976288 bytes read:21 seeks:1 nb_streams:1 > > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > > [lavf] mp_seek(0x51218090, 0, size) > > [lavf] 0=mp_read(0x51218090, 0x7fe4c7ce8800, 5000), pos: > > 538976288, eof:1 > > [lavf] 0=mp_read(0x51218090, 0x52d0a400, 32768), pos: 538976288, > > eof:1 > > [ffmpeg/audio] adpcm_psx: Decoder requires channel layout to be set > > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > > [lavf] mp_seek(0x51218090, 0, size) > > [lavf] mp_seek(0x51218090, 0, size) > > [lavf] mp_seek(0x51218090, 0, size) > > [ffmpeg/demuxer] vpk: stream 0: start_time: NOPTS duration: 0.069852 > > [ffmpeg/demuxer] vpk: format: start_time: NOPTS duration: 0.069852 > > (estimate from stream) bitrate=2 kb/s > > [ffmpeg/demuxer] vpk: Could not find codec parameters for stream 0 > > (Audio: adpcm_psx, 538976288 Hz, 0 channels): unspecified sample > > format > > [ffmpeg/demuxer] Consider increasing the value for the > > 'analyzeduration' (0) and 'probesize' (500) options > > [ffmpeg/demuxer] vpk: After avformat_find_stream_info() pos: 538976288 > > bytes read:21 seeks:1 frames:0 > > > > 3. the nb_channels value is cleared in avformat_find_stream_info() -> > > avcodec_parameters_from_context() -> codec_parameters_reset() and > > remains 0. > > This seems like the error: Why is AVCodecParameters being set from an > AVCodecContext if the codec could not be successfully opened? avcodec_open2() is only emitting a warning, no other action taken. https://github.com/FFmpeg/FFmpeg/blob/1b8d95da3a4a5c9441238928a36b653da693c286/libavformat/demux.c#L2603-L2605 later "some" things happen, but I think we could check if we have codec params before assigning them diff --git a/libavformat/demux.c b/libavformat/demux.c index dc65f9ad91..e8785304ca 100644 --- a/libavformat/demux.c +++ b/libavformat/demux.c @@ -3038,7 +3038,7 @@ int avformat_find_stream_info(AVFormatContext *ic, AVDictionary **options) AVStream *const st = ic->streams[i]; FFStream *const sti = ffstream(st); - if (sti->avctx_inited) { + if (sti->avctx_inited && has_codec_parameters(sti, NULL)) { ret = avcodec_parameters_from_context(st->codecpar, sti->avctx); if (ret < 0) goto find_stream_info_err; But the question is if we want to preserve old values in case of failed open or clear those values to indicate that really there are no valid parameters, because we failed to open the codec with current ones. - Kacper > > > > 4. as we can see there were errors, but it still returns success, so we > > proceed. > > > > 5. on the next av_seek_frame() which goes to vpk_read_packet() it will > > fail because nb_channels is 0 at this point. > > > > Sorry for only a high level overview, but at this moment, I'm not sure > > how
[FFmpeg-devel] [PATCH 4/6] fftools/cmdutils: split stream specifier parsing and matching
This approach has the major advantage that only parsing can fail (due to a malformed specifier or memory allocation failure). Since parsing is done generically, while matching is per-option, this will allow to remove substantial amounts of error checking code in following commits. The new code also explicitly allows stream specifiers to be followed by additional characters, which should allow cleaner handling of optional maps, i.e. -map ?, which is currently implemented in a hacky way that breaks when the stream specifier itself contains the '?' character (this can happen when matching metadata). It will also allow further extending the syntax, which will be useful in following commits. This introduces some minor behaviour changes: * Matching metadata tags now requires the ':' character in keys or values to be escaped. Previously it could not be present in keys, and would be used verbatim in values. The change is required in order to know where the value terminates. * Multiple stream types in a single specifier are now rejected - such a specifier makes no sense. * Non-existent stream group ID or index is now ignored with a warning rather than causing a failure. This is consistent with program handling and is required to make matching fail-free. --- fftools/cmdutils.c | 454 +++-- fftools/cmdutils.h | 56 ++ 2 files changed, 329 insertions(+), 181 deletions(-) diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index ffdcc85494..b3f501bd56 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -980,220 +980,312 @@ FILE *get_preset_file(char *filename, size_t filename_size, return f; } -/** - * Matches a stream specifier (but ignores requested index). - * - * @param indexptr set to point to the requested stream index if there is one - * - * @return <0 on error - * 0 if st is NOT a matching stream - * >0 if st is a matching stream - */ -static int match_stream_specifier(const AVFormatContext *s, const AVStream *st, - const char *spec, const char **indexptr, - const AVStreamGroup **g, const AVProgram **p) + +void stream_specifier_uninit(StreamSpecifier *ss) { -int match = 1; /* Stores if the specifier matches so far. */ +av_freep(&ss->meta_key); +av_freep(&ss->meta_val); +av_freep(&ss->remainder); + +memset(ss, 0, sizeof(*ss)); +} + +int stream_specifier_parse(StreamSpecifier *ss, const char *spec, + int allow_remainder, void *logctx) +{ +char *endptr; +int ret; + +memset(ss, 0, sizeof(*ss)); + +ss->idx = -1; +ss->media_type = AVMEDIA_TYPE_UNKNOWN; +ss->stream_list = STREAM_LIST_ALL; + +av_log(logctx, AV_LOG_TRACE, "Parsing stream specifier: %s\n", spec); + while (*spec) { if (*spec <= '9' && *spec >= '0') { /* opt:index */ -if (indexptr) -*indexptr = spec; -return match; +ss->idx = strtol(spec, &endptr, 0); + +av_assert0(endptr > spec); +spec = endptr; + +av_log(logctx, AV_LOG_TRACE, + "Parsed index: %d; remainder: %s\n", ss->idx, spec); + +// this terminates the specifier +break; } else if (*spec == 'v' || *spec == 'a' || *spec == 's' || *spec == 'd' || *spec == 't' || *spec == 'V') { /* opt:[vasdtV] */ -enum AVMediaType type; -int nopic = 0; +if (ss->media_type != AVMEDIA_TYPE_UNKNOWN) { +av_log(logctx, AV_LOG_ERROR, "Stream type specified multiple times\n"); +ret = AVERROR(EINVAL); +goto fail; +} switch (*spec++) { -case 'v': type = AVMEDIA_TYPE_VIDEO; break; -case 'a': type = AVMEDIA_TYPE_AUDIO; break; -case 's': type = AVMEDIA_TYPE_SUBTITLE; break; -case 'd': type = AVMEDIA_TYPE_DATA; break; -case 't': type = AVMEDIA_TYPE_ATTACHMENT; break; -case 'V': type = AVMEDIA_TYPE_VIDEO; nopic = 1; break; +case 'v': ss->media_type = AVMEDIA_TYPE_VIDEO; break; +case 'a': ss->media_type = AVMEDIA_TYPE_AUDIO; break; +case 's': ss->media_type = AVMEDIA_TYPE_SUBTITLE; break; +case 'd': ss->media_type = AVMEDIA_TYPE_DATA; break; +case 't': ss->media_type = AVMEDIA_TYPE_ATTACHMENT; break; +case 'V': ss->media_type = AVMEDIA_TYPE_VIDEO; + ss->no_apic= 1; break; default: av_assert0(0); } -if (*spec && *spec++ != ':') /* If we are not at the end, then another specifier must follow. */ -return AVERROR(EINVAL); -if (type != st->codecpar->codec_type) -match = 0; -
[FFmpeg-devel] [PATCH 2/6] fftools/ffmpeg: replace remaining uses of MATCH_PER_STREAM_OPT()
--- fftools/ffmpeg.h | 49 --- fftools/ffmpeg_demux.c| 39 - fftools/ffmpeg_mux_init.c | 71 --- fftools/ffmpeg_opt.c | 5 ++- 4 files changed, 91 insertions(+), 73 deletions(-) diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 7d82d7d7c2..674ae340f2 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -857,53 +857,16 @@ OutputStream *ost_iter(OutputStream *prev); void update_benchmark(const char *fmt, ...); -#define SPECIFIER_OPT_FMT_str "%s" -#define SPECIFIER_OPT_FMT_i"%i" -#define SPECIFIER_OPT_FMT_i64 "%"PRId64 -#define SPECIFIER_OPT_FMT_ui64 "%"PRIu64 -#define SPECIFIER_OPT_FMT_f"%f" -#define SPECIFIER_OPT_FMT_dbl "%lf" - -#define WARN_MULTIPLE_OPT_USAGE(optname, type, idx, st)\ -{\ -char namestr[128] = "";\ -const SpecifierOpt *so = &o->optname.opt[idx];\ -const char *spec = so->specifier && so->specifier[0] ? so->specifier : "";\ -snprintf(namestr, sizeof(namestr), "-%s", o->optname.opt_canon->name);\ -if (o->optname.opt_canon->flags & OPT_HAS_ALT) {\ -const char * const *names_alt = o->optname.opt_canon->u1.names_alt;\ -for (int _i = 0; names_alt[_i]; _i++)\ -av_strlcatf(namestr, sizeof(namestr), "/-%s", names_alt[_i]);\ -}\ -av_log(NULL, AV_LOG_WARNING, "Multiple %s options specified for stream %d, only the last option '-%s%s%s "SPECIFIER_OPT_FMT_##type"' will be used.\n",\ - namestr, st->index, o->optname.opt_canon->name, spec[0] ? ":" : "", spec, so->u.type);\ -} - -#define MATCH_PER_STREAM_OPT_CLEAN(name, type, outvar, fmtctx, st, clean)\ -{\ -int _ret, _matches = 0, _match_idx;\ -for (int _i = 0; _i < o->name.nb_opt; _i++) {\ -char *spec = o->name.opt[_i].specifier;\ -if ((_ret = check_stream_specifier(fmtctx, st, spec)) > 0) {\ -outvar = o->name.opt[_i].u.type;\ -_match_idx = _i;\ -_matches++;\ -} else if (_ret < 0)\ -clean;\ -}\ -if (_matches > 1 && o->name.opt_canon)\ - WARN_MULTIPLE_OPT_USAGE(name, type, _match_idx, st);\ -} - -#define MATCH_PER_STREAM_OPT(name, type, outvar, fmtctx, st)\ -{\ -MATCH_PER_STREAM_OPT_CLEAN(name, type, outvar, fmtctx, st, return _ret)\ -} - const char *opt_match_per_type_str(const SpecifierOptList *sol, char mediatype); int opt_match_per_stream_str(void *logctx, const SpecifierOptList *sol, AVFormatContext *fc, AVStream *st, const char **out); +int opt_match_per_stream_int(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, int *out); +int opt_match_per_stream_int64(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, int64_t *out); +int opt_match_per_stream_dbl(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, double *out); int muxer_thread(void *arg); int encoder_thread(void *arg); diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 0c92d31c10..e9a4b5c22a 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1153,11 +1153,18 @@ static int add_display_matrix_to_stream(const OptionsContext *o, double rotation = DBL_MAX; int hflip = -1, vflip = -1; int hflip_set = 0, vflip_set = 0, rotation_set = 0; +int ret; int32_t *buf; -MATCH_PER_STREAM_OPT(display_rotations, dbl, rotation, ctx, st); -MATCH_PER_STREAM_OPT(display_hflips,i, hflip,ctx, st); -MATCH_PER_STREAM_OPT(display_vflips,i, vflip,ctx, st); +ret = opt_match_per_stream_dbl(ist, &o->display_rotations, ctx, st, &rotation); +if (ret < 0) +return ret; +ret = opt_match_per_stream_int(ist, &o->display_hflips, ctx, st, &hflip); +if (ret < 0) +return ret; +ret = opt_match_per_stream_int(ist, &o->display_vflips, ctx, st, &vflip); +if (ret < 0) +return ret; rotation_set = rotation != DBL_MAX; hflip_set= hflip != -1; @@ -1255,10 +1262,14 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st, AVDictiona ds->dec_opts.time_base = st->time_base; ds->ts_scale = 1.0; -MATCH_PER_STREAM_OPT(ts_scale, dbl, ds->ts_scale, ic, st); +ret = opt_match_per_stream_dbl(ist, &o->ts_scale, ic, st, &ds->ts_scale); +if (ret < 0) +return ret; ds->autorotate = 1; -MATCH_PER_STREAM_OPT(autorotate, i, ds->autorotate, ic, st); +ret = opt_match_per_stream_int(ist, &o->autorotate, ic, st, &ds->autorotate); +if (ret < 0) +return ret; ds->apply_cropping = CROP_ALL; ret = opt_match_per_stream_str(ist, &o->apply_cropping, ic, st, &apply_cropping); @@ -1397,7 +1408,9 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st, AVDictiona } ds->reinit_filter
[FFmpeg-devel] [PATCH 6/6] fftools/ffmpeg: switch -map parsing to new stream specifier API
Makes optional map handling less hacky, fixes combining optional maps with metadata matching on tags/values containing the '?' character/ Forward errors from stream specifier parsing, previously the code would ignore them. --- fftools/ffmpeg_opt.c | 54 ++-- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index f6df6ae952..3cbf0795ac 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -427,22 +427,20 @@ static int opt_map(void *optctx, const char *opt, const char *arg) { OptionsContext *o = optctx; StreamMap *m = NULL; +StreamSpecifier ss; int i, negative = 0, file_idx, disabled = 0; -int ret; -char *map, *p; -char *allow_unused; +int ret, allow_unused = 0; + +memset(&ss, 0, sizeof(ss)); if (*arg == '-') { negative = 1; arg++; } -map = av_strdup(arg); -if (!map) -return AVERROR(ENOMEM); -if (map[0] == '[') { +if (arg[0] == '[') { /* this mapping refers to lavfi output */ -const char *c = map + 1; +const char *c = arg + 1; ret = GROW_ARRAY(o->stream_maps, o->nb_stream_maps); if (ret < 0) @@ -451,33 +449,55 @@ static int opt_map(void *optctx, const char *opt, const char *arg) m = &o->stream_maps[o->nb_stream_maps - 1]; m->linklabel = av_get_token(&c, "]"); if (!m->linklabel) { -av_log(NULL, AV_LOG_ERROR, "Invalid output link label: %s.\n", map); +av_log(NULL, AV_LOG_ERROR, "Invalid output link label: %s.\n", arg); ret = AVERROR(EINVAL); goto fail; } } else { -if (allow_unused = strchr(map, '?')) -*allow_unused = 0; -file_idx = strtol(map, &p, 0); +char *endptr; + +file_idx = strtol(arg, &endptr, 0); if (file_idx >= nb_input_files || file_idx < 0) { av_log(NULL, AV_LOG_FATAL, "Invalid input file index: %d.\n", file_idx); ret = AVERROR(EINVAL); goto fail; } +arg = endptr; + +ret = stream_specifier_parse(&ss, *arg == ':' ? arg + 1 : arg, 1, NULL); +if (ret < 0) { +av_log(NULL, AV_LOG_ERROR, "Invalid stream specifier: %s\n", arg); +goto fail; +} + +if (ss.remainder) { +if (!strcmp(ss.remainder, "?")) +allow_unused = 1; +else { +av_log(NULL, AV_LOG_ERROR, "Trailing garbage after stream specifier: %s\n", + ss.remainder); +ret = AVERROR(EINVAL); +goto fail; +} +} + if (negative) /* disable some already defined maps */ for (i = 0; i < o->nb_stream_maps; i++) { m = &o->stream_maps[i]; if (file_idx == m->file_index && -check_stream_specifier(input_files[m->file_index]->ctx, +stream_specifier_match(&ss, + input_files[m->file_index]->ctx, input_files[m->file_index]->ctx->streams[m->stream_index], - *p == ':' ? p + 1 : p) > 0) + NULL)) m->disabled = 1; } else for (i = 0; i < input_files[file_idx]->nb_streams; i++) { -if (check_stream_specifier(input_files[file_idx]->ctx, input_files[file_idx]->ctx->streams[i], -*p == ':' ? p + 1 : p) <= 0) +if (!stream_specifier_match(&ss, +input_files[file_idx]->ctx, + input_files[file_idx]->ctx->streams[i], +NULL)) continue; if (input_files[file_idx]->streams[i]->user_set_discard == AVDISCARD_ALL) { disabled = 1; @@ -511,7 +531,7 @@ static int opt_map(void *optctx, const char *opt, const char *arg) } ret = 0; fail: -av_freep(&map); +stream_specifier_uninit(&ss); return ret; } -- 2.43.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 3/6] fftools/cmdutils: put stream specifier handling back into cmdutils
Stream specifiers were originally designed exclusively for CLI use and were not intended to be public API. Handling them in avformat places major restrictions on how they are used. E.g. if ffmpeg CLI wishes to override some stream parameters, it has to change the demuxer fields (since avformat_match_stream_specifier() does not have access to anything else). However, such fields are supposed to be read-only for the caller. Furthermore having this code in avformat restricts extending the specifier syntax. An example of such an extension will be added in following commits. --- fftools/cmdutils.c | 211 - 1 file changed, 210 insertions(+), 1 deletion(-) diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 6286fd87f7..ffdcc85494 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -980,10 +980,219 @@ FILE *get_preset_file(char *filename, size_t filename_size, return f; } +/** + * Matches a stream specifier (but ignores requested index). + * + * @param indexptr set to point to the requested stream index if there is one + * + * @return <0 on error + * 0 if st is NOT a matching stream + * >0 if st is a matching stream + */ +static int match_stream_specifier(const AVFormatContext *s, const AVStream *st, + const char *spec, const char **indexptr, + const AVStreamGroup **g, const AVProgram **p) +{ +int match = 1; /* Stores if the specifier matches so far. */ +while (*spec) { +if (*spec <= '9' && *spec >= '0') { /* opt:index */ +if (indexptr) +*indexptr = spec; +return match; +} else if (*spec == 'v' || *spec == 'a' || *spec == 's' || *spec == 'd' || + *spec == 't' || *spec == 'V') { /* opt:[vasdtV] */ +enum AVMediaType type; +int nopic = 0; + +switch (*spec++) { +case 'v': type = AVMEDIA_TYPE_VIDEO; break; +case 'a': type = AVMEDIA_TYPE_AUDIO; break; +case 's': type = AVMEDIA_TYPE_SUBTITLE; break; +case 'd': type = AVMEDIA_TYPE_DATA; break; +case 't': type = AVMEDIA_TYPE_ATTACHMENT; break; +case 'V': type = AVMEDIA_TYPE_VIDEO; nopic = 1; break; +default: av_assert0(0); +} +if (*spec && *spec++ != ':') /* If we are not at the end, then another specifier must follow. */ +return AVERROR(EINVAL); + +if (type != st->codecpar->codec_type) +match = 0; +if (nopic && (st->disposition & AV_DISPOSITION_ATTACHED_PIC)) +match = 0; +} else if (*spec == 'g' && *(spec + 1) == ':') { +int64_t group_idx = -1, group_id = -1; +int found = 0; +char *endptr; +spec += 2; +if (*spec == '#' || (*spec == 'i' && *(spec + 1) == ':')) { +spec += 1 + (*spec == 'i'); +group_id = strtol(spec, &endptr, 0); +if (spec == endptr || (*endptr && *endptr++ != ':')) +return AVERROR(EINVAL); +spec = endptr; +} else { +group_idx = strtol(spec, &endptr, 0); +/* Disallow empty id and make sure that if we are not at the end, then another specifier must follow. */ +if (spec == endptr || (*endptr && *endptr++ != ':')) +return AVERROR(EINVAL); +spec = endptr; +} +if (match) { +if (group_id > 0) { +for (unsigned i = 0; i < s->nb_stream_groups; i++) { +if (group_id == s->stream_groups[i]->id) { +group_idx = i; +break; +} +} +} +if (group_idx < 0 || group_idx >= s->nb_stream_groups) +return AVERROR(EINVAL); +for (unsigned j = 0; j < s->stream_groups[group_idx]->nb_streams; j++) { +if (st->index == s->stream_groups[group_idx]->streams[j]->index) { +found = 1; +if (g) +*g = s->stream_groups[group_idx]; +break; +} +} +} +if (!found) +match = 0; +} else if (*spec == 'p' && *(spec + 1) == ':') { +int prog_id; +int found = 0; +char *endptr; +spec += 2; +prog_id = strtol(spec, &endptr, 0); +/* Disallow empty id and make sure that if we are not at the end, then another specifier must follow. */ +if (spec == endptr || (*endptr && *endptr++ != ':')) +return AVERROR(EINVAL); +spec =
[FFmpeg-devel] [PATCH 5/6] fftools/ffmpeg: use new stream specifier API in opt_match_per_stream*()
Removes a lot of error checking code, as matching cannot fail. --- fftools/cmdutils.c| 8 ++ fftools/cmdutils.h| 6 +- fftools/ffmpeg.h | 16 +-- fftools/ffmpeg_demux.c| 87 - fftools/ffmpeg_mux_init.c | 198 ++ fftools/ffmpeg_opt.c | 32 +++--- 6 files changed, 105 insertions(+), 242 deletions(-) diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index b3f501bd56..f725c31c12 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -288,6 +288,14 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, goto finish; } sol->opt[sol->nb_opt - 1].specifier = str; + +if (po->flags & OPT_FLAG_PERSTREAM) { +ret = stream_specifier_parse(&sol->opt[sol->nb_opt - 1].stream_spec, + str, 0, NULL); +if (ret < 0) +goto finish; +} + dst = &sol->opt[sol->nb_opt - 1].u; } diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h index f7005aabf9..9609c6c739 100644 --- a/fftools/cmdutils.h +++ b/fftools/cmdutils.h @@ -159,7 +159,11 @@ unsigned stream_specifier_match(const StreamSpecifier *ss, void stream_specifier_uninit(StreamSpecifier *ss); typedef struct SpecifierOpt { -char *specifier;/**< stream/chapter/program/... specifier */ +// original specifier or empty string +char*specifier; +// parsed specifier for OPT_FLAG_PERSTREAM options +StreamSpecifier stream_spec; + union { uint8_t *str; inti; diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index 674ae340f2..3c5d933e17 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -859,14 +859,14 @@ void update_benchmark(const char *fmt, ...); const char *opt_match_per_type_str(const SpecifierOptList *sol, char mediatype); -int opt_match_per_stream_str(void *logctx, const SpecifierOptList *sol, - AVFormatContext *fc, AVStream *st, const char **out); -int opt_match_per_stream_int(void *logctx, const SpecifierOptList *sol, - AVFormatContext *fc, AVStream *st, int *out); -int opt_match_per_stream_int64(void *logctx, const SpecifierOptList *sol, - AVFormatContext *fc, AVStream *st, int64_t *out); -int opt_match_per_stream_dbl(void *logctx, const SpecifierOptList *sol, - AVFormatContext *fc, AVStream *st, double *out); +void opt_match_per_stream_str(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, const char **out); +void opt_match_per_stream_int(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, int *out); +void opt_match_per_stream_int64(void *logctx, const SpecifierOptList *sol, +AVFormatContext *fc, AVStream *st, int64_t *out); +void opt_match_per_stream_dbl(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, double *out); int muxer_thread(void *arg); int encoder_thread(void *arg); diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index e9a4b5c22a..039ee0c785 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1084,12 +1084,8 @@ static int choose_decoder(const OptionsContext *o, void *logctx, { const char *codec_name = NULL; -int ret; - -ret = opt_match_per_stream_str(logctx, &o->codec_names, s, st, &codec_name); -if (ret < 0) -return ret; +opt_match_per_stream_str(logctx, &o->codec_names, s, st, &codec_name); if (codec_name) { int ret = find_codec(NULL, codec_name, st->codecpar->codec_type, 0, pcodec); if (ret < 0) @@ -1153,18 +1149,11 @@ static int add_display_matrix_to_stream(const OptionsContext *o, double rotation = DBL_MAX; int hflip = -1, vflip = -1; int hflip_set = 0, vflip_set = 0, rotation_set = 0; -int ret; int32_t *buf; -ret = opt_match_per_stream_dbl(ist, &o->display_rotations, ctx, st, &rotation); -if (ret < 0) -return ret; -ret = opt_match_per_stream_int(ist, &o->display_hflips, ctx, st, &hflip); -if (ret < 0) -return ret; -ret = opt_match_per_stream_int(ist, &o->display_vflips, ctx, st, &vflip); -if (ret < 0) -return ret; +opt_match_per_stream_dbl(ist, &o->display_rotations, ctx, st, &rotation); +opt_match_per_stream_int(ist, &o->display_hflips, ctx, st, &hflip); +opt_match_per_stream_int(ist, &o->display_vflips, ctx, st, &vflip); rotation_set = rotation != DBL_MAX; hflip_set= hflip != -1; @@ -1262,19 +1251,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st, AVDictiona ds->dec_opts.time_base = st->time_base; ds->ts_scale = 1.0; -ret = opt_match_
[FFmpeg-devel] [PATCH 1/6] fftools/ffmpeg: replace MATCH_PER_STREAM_OPT(.., str, ..) with a function
This has multiple advantages: * The macro has multiple parameters that often have similar or identical values, yet very different meanings (one is the name of the OptionsContext member where the parsed options are stored, the other the name of the variable into which the result is written); this change makes each of these explicit. * The macro returns on failure, which may cause leaks - this was the reason for adding MATCH_PER_STREAM_OPT_CLEAN(), also ost_add() currently leaks encoder_opts. The new function returns failure to its caller, which decides how to deal with it. While that adds a lot of error checks/forwards for now, those will be reduced in following commits. * new code is type- and const- correct Invocations of MATCH_PER_STREAM_OPT() with other types will be converted in following commits. --- fftools/cmdutils.c| 6 +- fftools/cmdutils.h| 3 + fftools/ffmpeg.h | 4 +- fftools/ffmpeg_demux.c| 74 -- fftools/ffmpeg_mux.h | 2 +- fftools/ffmpeg_mux_init.c | 156 +++--- fftools/ffmpeg_opt.c | 67 7 files changed, 242 insertions(+), 70 deletions(-) diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 9b18cf5e4d..6286fd87f7 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -246,6 +246,8 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, (uint8_t *)optctx + po->u.off : po->u.dst_ptr; char *arg_allocated = NULL; +enum OptionType so_type = po->type; + SpecifierOptList *sol = NULL; double num; int ret = 0; @@ -310,6 +312,7 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, goto finish; *(int *)dst = num; +so_type = OPT_TYPE_INT; } else if (po->type == OPT_TYPE_INT64) { ret = parse_number(opt, arg, OPT_TYPE_INT64, INT64_MIN, (double)INT64_MAX, &num); if (ret < 0) @@ -323,6 +326,7 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, opt, arg); goto finish; } +so_type = OPT_TYPE_INT64; } else if (po->type == OPT_TYPE_FLOAT) { ret = parse_number(opt, arg, OPT_TYPE_FLOAT, -INFINITY, INFINITY, &num); if (ret < 0) @@ -352,7 +356,7 @@ static int write_option(void *optctx, const OptionDef *po, const char *opt, } if (sol) { -sol->type = po->type; +sol->type = so_type; sol->opt_canon = (po->flags & OPT_HAS_CANON) ? find_option(defs, po->u1.name_canon) : po; } diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h index 14340dff7d..abc8d26607 100644 --- a/fftools/cmdutils.h +++ b/fftools/cmdutils.h @@ -120,6 +120,9 @@ typedef struct SpecifierOptList { /* Canonical option definition that was parsed into this list. */ const struct OptionDef *opt_canon; +/* Type corresponding to the field that should be used from SpecifierOpt.u. + * May not match the option type, e.g. OPT_TYPE_BOOL options are stored as + * int, so this field would be OPT_TYPE_INT for them */ enum OptionType type; } SpecifierOptList; diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index d0298d53cf..7d82d7d7c2 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -593,7 +593,7 @@ typedef struct OutputStream { KeyframeForceCtx kf; -char *logfile_prefix; +const char *logfile_prefix; FILE *logfile; // simple filtergraph feeding this stream, if any @@ -902,6 +902,8 @@ void update_benchmark(const char *fmt, ...); const char *opt_match_per_type_str(const SpecifierOptList *sol, char mediatype); +int opt_match_per_stream_str(void *logctx, const SpecifierOptList *sol, + AVFormatContext *fc, AVStream *st, const char **out); int muxer_thread(void *arg); int encoder_thread(void *arg); diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c index 6f7d78c896..0c92d31c10 100644 --- a/fftools/ffmpeg_demux.c +++ b/fftools/ffmpeg_demux.c @@ -1077,14 +1077,19 @@ int ist_filter_add(InputStream *ist, InputFilter *ifilter, int is_simple, return ds->sch_idx_dec; } -static int choose_decoder(const OptionsContext *o, AVFormatContext *s, AVStream *st, +static int choose_decoder(const OptionsContext *o, void *logctx, + AVFormatContext *s, AVStream *st, enum HWAccelID hwaccel_id, enum AVHWDeviceType hwaccel_device_type, const AVCodec **pcodec) { -char *codec_name = NULL; +const char *codec_name = NULL; +int ret; + +ret = opt_match_per_stream_str(logctx, &o->codec_names, s, st, &codec_name); +if (ret < 0) +return ret; -MATCH_PER_STREAM_OPT(codec_names, str, codec_name, s, st); if (codec_name) { int ret = find_codec(NULL, codec_name, st
[FFmpeg-devel] 回复: [PATCH v2 2/3] avcodec/vvc/cabac: remove vvc_refill2
From Andreas Rheinhardt: > 发件人: ffmpeg-devel 代表 Andreas Rheinhardt > > 发送时间: 2024年8月8日 13:19 > 收件人: ffmpeg-devel@ffmpeg.org > 主题: Re: [FFmpeg-devel] [PATCH v2 2/3] avcodec/vvc/cabac: remove vvc_refill2 > > toq...@outlook.com: >> From: Wu Jianhua >> >> See https://github.com/ffvvc/FFmpeg/issues/178 > > This link only sends one to a patchwork thread to read. The commit > message should instead explain why this is done on its own (and may > refer to the mailing list thread for a more detailed explanation). > > Same for the next patch. > > No problem. I will send a new patchset. Thanks, Jianhua ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v3 1/3] avcodec/vvc_parser: move avctx->has_b_frames initialization to dec
From: Wu Jianhua >From Jun Zhao : > Should we relocate this to the decoder? Other codecs typically set this > parameter in the decoder. Signed-off-by: Wu Jianhua --- libavcodec/vvc/dec.c| 1 + libavcodec/vvc_parser.c | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c index d04f68e4cf..6e225d278a 100644 --- a/libavcodec/vvc/dec.c +++ b/libavcodec/vvc/dec.c @@ -748,6 +748,7 @@ static void export_frame_params(VVCContext *s, const VVCFrameContext *fc) c->coded_height = pps->height; c->width= pps->width - ((pps->r->pps_conf_win_left_offset + pps->r->pps_conf_win_right_offset) << sps->hshift[CHROMA]); c->height = pps->height - ((pps->r->pps_conf_win_top_offset + pps->r->pps_conf_win_bottom_offset) << sps->vshift[CHROMA]); +c->has_b_frames = sps->r->sps_dpb_params.dpb_max_num_reorder_pics[sps->r->sps_max_sublayers_minus1]; } static int frame_setup(VVCFrameContext *fc, VVCContext *s) diff --git a/libavcodec/vvc_parser.c b/libavcodec/vvc_parser.c index 5373875aae..8d32d66573 100644 --- a/libavcodec/vvc_parser.c +++ b/libavcodec/vvc_parser.c @@ -185,9 +185,6 @@ static void set_parser_ctx(AVCodecParserContext *s, AVCodecContext *avctx, avctx->color_range = sps->vui.vui_full_range_flag ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; -avctx->has_b_frames = - sps->sps_dpb_params.dpb_max_num_reorder_pics[sps->sps_max_sublayers_minus1]; - if (sps->sps_ptl_dpb_hrd_params_present_flag && sps->sps_timing_hrd_params_present_flag) { uint32_t num = sps->sps_general_timing_hrd_parameters.num_units_in_tick; -- 2.44.0.windows.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 v3 2/3] avcodec/vvc/cabac: remove vvc_refill2
From: Wu Jianhua The vvc_refill2 is the same as the refill2 in cabac_functions. Remove it to reduce duplicated codes. Signed-off-by: Wu Jianhua --- libavcodec/cabac_functions.h | 2 +- libavcodec/vvc/cabac.c | 28 +--- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/libavcodec/cabac_functions.h b/libavcodec/cabac_functions.h index c3f08d3410..9bee401f2c 100644 --- a/libavcodec/cabac_functions.h +++ b/libavcodec/cabac_functions.h @@ -85,7 +85,7 @@ static inline void renorm_cabac_decoder_once(CABACContext *c){ } #endif -#ifndef get_cabac_inline +#if !defined(get_cabac_inline) || !defined(refill2) static void refill2(CABACContext *c){ int i; unsigned x; diff --git a/libavcodec/vvc/cabac.c b/libavcodec/vvc/cabac.c index 0d45eec751..c9b6f9bf3e 100644 --- a/libavcodec/vvc/cabac.c +++ b/libavcodec/vvc/cabac.c @@ -856,32 +856,6 @@ int ff_vvc_cabac_init(VVCLocalContext *lc, return ret; } -//fixme -static void vvc_refill2(CABACContext* c) { -int i; -unsigned x; -#if !HAVE_FAST_CLZ -x = c->low ^ (c->low - 1); -i = 7 - ff_h264_norm_shift[x >> (CABAC_BITS - 1)]; -#else -i = ff_ctz(c->low) - CABAC_BITS; -#endif - -x = -CABAC_MASK; - -#if CABAC_BITS == 16 -x += (c->bytestream[0] << 9) + (c->bytestream[1] << 1); -#else -x += c->bytestream[0] << 1; -#endif - -c->low += x << i; -#if !UNCHECKED_BITSTREAM_READER -if (c->bytestream < c->bytestream_end) -#endif -c->bytestream += CABAC_BITS / 8; -} - static int inline vvc_get_cabac(CABACContext *c, VVCCabacState* base, const int ctx) { VVCCabacState *s = base + ctx; @@ -904,7 +878,7 @@ static int inline vvc_get_cabac(CABACContext *c, VVCCabacState* base, const int c->low <<= lps_mask; if (!(c->low & CABAC_MASK)) -vvc_refill2(c); +refill2(c); s->state[0] = s->state[0] - (s->state[0] >> s->shift[0]) + (1023 * bit >> s->shift[0]); s->state[1] = s->state[1] - (s->state[1] >> s->shift[1]) + (16383 * bit >> s->shift[1]); return bit; -- 2.44.0.windows.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 v3 3/3] avcodec/vvc/dsp: prefix TxType and TxSize with VVC
From: Wu Jianhua See https://patchwork.ffmpeg.org/project/ffmpeg/patch/tyspr06mb64337c4a9adf5312e6648543aa...@tyspr06mb6433.apcprd06.prod.outlook.com/#81892 Signed-off-by: Wu Jianhua --- libavcodec/vvc/dsp.h | 28 ++-- libavcodec/vvc/dsp_template.c | 2 +- libavcodec/vvc/intra.c| 26 +- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/libavcodec/vvc/dsp.h b/libavcodec/vvc/dsp.h index 0b49b97021..38ff492a23 100644 --- a/libavcodec/vvc/dsp.h +++ b/libavcodec/vvc/dsp.h @@ -27,21 +27,21 @@ #include #include -enum TxType { -DCT2, -DST7, -DCT8, -N_TX_TYPE, +enum VVCTxType { +VVC_DCT2, +VVC_DST7, +VVC_DCT8, +VVC_N_TX_TYPE, }; -enum TxSize { -TX_SIZE_2, -TX_SIZE_4, -TX_SIZE_8, -TX_SIZE_16, -TX_SIZE_32, -TX_SIZE_64, -N_TX_SIZE, +enum VVCTxSize { +VVC_TX_SIZE_2, +VVC_TX_SIZE_4, +VVC_TX_SIZE_8, +VVC_TX_SIZE_16, +VVC_TX_SIZE_32, +VVC_TX_SIZE_64, +VVC_N_TX_SIZE, }; typedef struct VVCInterDSPContext { @@ -127,7 +127,7 @@ typedef struct VVCItxDSPContext { void (*add_residual_joint)(uint8_t *dst, const int *res, int width, int height, ptrdiff_t stride, int c_sign, int shift); void (*pred_residual_joint)(int *buf, int width, int height, int c_sign, int shift); -void (*itx[N_TX_TYPE][N_TX_SIZE])(int *coeffs, ptrdiff_t step, size_t nz); +void (*itx[VVC_N_TX_TYPE][VVC_N_TX_SIZE])(int *coeffs, ptrdiff_t step, size_t nz); void (*transform_bdpcm)(int *coeffs, int width, int height, int vertical, int log2_transform_range); } VVCItxDSPContext; diff --git a/libavcodec/vvc/dsp_template.c b/libavcodec/vvc/dsp_template.c index 8130abbccf..1aa1e027bd 100644 --- a/libavcodec/vvc/dsp_template.c +++ b/libavcodec/vvc/dsp_template.c @@ -97,7 +97,7 @@ static void FUNC(transform_bdpcm)(int *coeffs, const int width, const int height static void FUNC(ff_vvc_itx_dsp_init)(VVCItxDSPContext *const itx) { #define VVC_ITX(TYPE, type, s) \ -itx->itx[TYPE][TX_SIZE_##s] = ff_vvc_inv_##type##_##s; \ +itx->itx[VVC_##TYPE][VVC_##TX_SIZE_##s] = ff_vvc_inv_##type##_##s; \ #define VVC_ITX_COMMON(TYPE, type) \ VVC_ITX(TYPE, type, 4); \ diff --git a/libavcodec/vvc/intra.c b/libavcodec/vvc/intra.c index f77a012f09..73dca6dc85 100644 --- a/libavcodec/vvc/intra.c +++ b/libavcodec/vvc/intra.c @@ -128,15 +128,15 @@ static void ilfnst_transform(const VVCLocalContext *lc, TransformBlock *tb) } //part of 8.7.4 Transformation process for scaled transform coefficients -static void derive_transform_type(const VVCFrameContext *fc, const VVCLocalContext *lc, const TransformBlock *tb, enum TxType *trh, enum TxType *trv) +static void derive_transform_type(const VVCFrameContext *fc, const VVCLocalContext *lc, const TransformBlock *tb, enum VVCTxType *trh, enum VVCTxType *trv) { const CodingUnit *cu = lc->cu; -static const enum TxType mts_to_trh[] = {DCT2, DST7, DCT8, DST7, DCT8}; -static const enum TxType mts_to_trv[] = {DCT2, DST7, DST7, DCT8, DCT8}; +static const enum VVCTxType mts_to_trh[] = { VVC_DCT2, VVC_DST7, VVC_DCT8, VVC_DST7, VVC_DCT8 }; +static const enum VVCTxType mts_to_trv[] = { VVC_DCT2, VVC_DST7, VVC_DST7, VVC_DCT8, VVC_DCT8 }; const VVCSPS *sps = fc->ps.sps; int implicit_mts_enabled = 0; if (tb->c_idx || (cu->isp_split_type != ISP_NO_SPLIT && cu->lfnst_idx)) { -*trh = *trv = DCT2; +*trh = *trv = VVC_DCT2; return; } @@ -152,11 +152,11 @@ static void derive_transform_type(const VVCFrameContext *fc, const VVCLocalConte const int w = tb->tb_width; const int h = tb->tb_height; if (cu->sbt_flag) { -*trh = (cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? DST7 : DCT8; -*trv = (!cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? DST7 : DCT8; +*trh = (cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? VVC_DST7 : VVC_DCT8; +*trv = (!cu->sbt_horizontal_flag || cu->sbt_pos_flag) ? VVC_DST7 : VVC_DCT8; } else { -*trh = (w >= 4 && w <= 16) ? DST7 : DCT2; -*trv = (h >= 4 && h <= 16) ? DST7 : DCT2; +*trh = (w >= 4 && w <= 16) ? VVC_DST7 : VVC_DCT2; +*trv = (h >= 4 && h <= 16) ? VVC_DST7 : VVC_DCT2; } return; } @@ -447,7 +447,7 @@ static void dequant(const VVCLocalContext *lc, const TransformUnit *tu, Transfor //transmatrix[0][0] #define DCT_A 64 -static void itx_2d(const VVCFrameContext *fc, TransformBlock *tb, const enum TxType trh, const enum TxType trv) +static void itx_2d(const VVCFrameContext *fc, TransformBlock *tb, const enum VVCTxType trh, const enum VVCTxType trv) { const VVCSPS *sps = fc->ps
[FFmpeg-devel] [PATCH] tests/iamf: match stream group by id in some tests
Increases specifier parsing code coverage a little bit. Signed-off-by: James Almer --- tests/fate/iamf.mak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fate/iamf.mak b/tests/fate/iamf.mak index 164fd78bf6..bbf465bec6 100644 --- a/tests/fate/iamf.mak +++ b/tests/fate/iamf.mak @@ -39,12 +39,12 @@ fate-iamf-ambisonic_1: CMD = transcode wav $(SRC) iamf "-auto_conversion_filters FATE_IAMF_SAMPLES-$(call FRAMECRC, IAMF, OPUS) += fate-iamf-5_1-demux fate-iamf-5_1-demux: CMD = stream_demux iamf $(TARGET_SAMPLES)/iamf/test_59.iamf "" \ - "-c:a copy -frames:a 0 -map 0" \ + "-c:a copy -frames:a 0 -map 0:g:\#42" \ "-show_entries stream_group=index,id,nb_streams,type:stream_group_components:stream_group_stream=index,id:stream_group_stream_disposition" FATE_IAMF_SAMPLES-$(call REMUX, IAMF, OPUS_DECODER) += fate-iamf-5_1-copy fate-iamf-5_1-copy: CMD = stream_remux iamf $(TARGET_SAMPLES)/iamf/test_59.iamf "" iamf \ - "-map 0 -stream_group map=0=0:st=0:st=1:st=2:st=3 -stream_group map=0=1:stg=0 -streamid 0:0 -streamid 1:1 -streamid 2:2 -streamid 3:3" "" "-c:a copy -frames:a 0 -map 0" \ + "-map 0 -stream_group map=0=0:st=0:st=1:st=2:st=3 -stream_group map=0=1:stg=0 -streamid 0:0 -streamid 1:1 -streamid 2:2 -streamid 3:3" "" "-c:a copy -frames:a 0 -map 0:g:i:42" \ "-show_entries stream_group=index,id,nb_streams,type:stream_group_components:stream_group_stream=index,id:stream_group_stream_disposition" FATE_IAMF += $(FATE_IAMF-yes) -- 2.46.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] avfilter/video: don't zero allocated buffers if memory poisoning is used
Same as in avcodec/get_buffer.c Should help in debugging use of uninitialized memory. Signed-off-by: James Almer --- libavfilter/video.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/libavfilter/video.c b/libavfilter/video.c index 89d0797ab5..afd3dc3dc3 100644 --- a/libavfilter/video.c +++ b/libavfilter/video.c @@ -71,8 +71,10 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig } if (!li->frame_pool) { -li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h, - link->format, align); +li->frame_pool = ff_frame_pool_video_init(CONFIG_MEMORY_POISONING + ? NULL + : av_buffer_allocz, + w, h, link->format, align); if (!li->frame_pool) return NULL; } else { @@ -86,8 +88,10 @@ AVFrame *ff_default_get_video_buffer2(AVFilterLink *link, int w, int h, int alig pool_format != link->format || pool_align != align) { ff_frame_pool_uninit(&li->frame_pool); -li->frame_pool = ff_frame_pool_video_init(av_buffer_allocz, w, h, - link->format, align); +li->frame_pool = ff_frame_pool_video_init(CONFIG_MEMORY_POISONING + ? NULL + : av_buffer_allocz, + w, h, link->format, align); if (!li->frame_pool) return NULL; } -- 2.46.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".