Re: [FFmpeg-devel] [PATCH 000/279 v2] New channel layout API
On Sat, 18 Dec 2021, Michael Niedermayer wrote: On Sat, Dec 18, 2021 at 02:36:12PM +0100, Michael Niedermayer wrote: On Fri, Dec 17, 2021 at 07:04:08PM +0100, Marton Balint wrote: On Fri, 17 Dec 2021, Michael Niedermayer wrote: On Fri, Dec 17, 2021 at 01:04:19AM +0100, Marton Balint wrote: On Thu, 16 Dec 2021, James Almer wrote: Resending the first two patches only, since this is meant to show the implementation of one of the several suggestions made in the previous set that need to be discussed and hopefully resolved in a call. Can you push the full branch somewhere? The proposals so far to extend the API to support either custom labels for channels are, or some form of extra user information. - Fixed array of bytes to hold a label. Simple solution, but the labels will have a hard limit that can only be extended with a major bump. This is what i implemented in this version. - "char *name" per channel that the user may allocate and the API will manage, duplicate and free. Simple solution, and the name can be arbitrarily long, but inefficient (av_strdup() per channel with a custom label on layout copy). - "const char *name" per channel for compile time constants, or that the user may allocate and free. Very efficient, but for non compile time strings ensuring they outlive the layout can be tricky. - Refcounted AVChannelCustom with a dictionary. This can't be done with AVBufferRef, so it would require some other form of reference counting. And a dictionary may add quite a bit of complexity to the API, as you can set anything on them. Until we have proper refcounting API we can make the AVBufferRef in AVChannelLayout a void *, and only allow channel_layout functions to dereference it as an AVBufferRef. This would mean adding some extra helper functions to channel layout, but overall it is not unsolvable. The real question is that if you want to use refcounting and add helpers to query / replace per-channel metadata, or you find the idea too heavy weight and would like to stick to flat structs. what is the advantage of refcounting for channel metadata ? is it about the used memory, about the reduced need to copy ? Basicly it is the ability to store per-channel metadata in avdictionary, because otherwise it would have to be copyed, and avdictionary is very ineffective at copying because of many mallocs. what kind of metadata and what size do you expect ? bytes, kilobytes, megabytes, gigabytes per channel ? Usually, nothing, because most format don't have support for per-channel metadata. In some cases it is going to be a couple of textual metadata key-value pairs, such as language, label, group, speaker, positon, so 4-5 dynamically allocated string pairs, plus the AVDictionary itself, multiplied by the number of channels in a layout. what is the overhead for dynamic allocation and ref counting? that is at which point does it even make sense ? I don't have exact measurements. It is generally felt that copying AVDictionary per-channel is a huge overhead for something as lightweight as an audio frame which is a 2-4 kB per channel at most and only a couple of allocs usually not dependant on the number of channels. That's why refcounting was proposed. I was thinking more at a AVStream / AVCodecParameters level. How will a demuxer transport such metadata over a AVPacket into a decoder outputting metadata-filled AVFrames? or is this never needed ? I am not sure I understand. Usually metadata is passed from demuxer to decoder by avcodec_parameters_to_context(), this is used for all metadata which is in AVCodecParameters. For per-packet metadata ff_decode_frame_props() has some automatic packet side data -> frame side data transfer. AVStream side data may be transferred to AVPacket side data if av_format_inject_global_side_data() is used, but it is not enabled by default. 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 000/279 v2] New channel layout API
On Sun, Dec 19, 2021 at 12:35:11PM +0100, Marton Balint wrote: > > > On Sat, 18 Dec 2021, Michael Niedermayer wrote: > > > On Sat, Dec 18, 2021 at 02:36:12PM +0100, Michael Niedermayer wrote: > > > On Fri, Dec 17, 2021 at 07:04:08PM +0100, Marton Balint wrote: > > > > > > > > > > > > On Fri, 17 Dec 2021, Michael Niedermayer wrote: > > > > > > > > > On Fri, Dec 17, 2021 at 01:04:19AM +0100, Marton Balint wrote: > > > > > > > > > > > > > > > > > > On Thu, 16 Dec 2021, James Almer wrote: > > > > > > > > > > > > > Resending the first two patches only, since this is meant to > > > > > > > show the implementation of one of the several suggestions made > > > > > > > in the previous set that need to be discussed and hopefully > > > > > > > resolved in a call. > > > > > > > > > > > > Can you push the full branch somewhere? > > > > > > > > > > > > > > > > > > > > The proposals so far to extend the API to support either custom > > > > > > > labels for channels are, or some form of extra user information. > > > > > > > > > > > > > > - Fixed array of bytes to hold a label. Simple solution, but > > > > > > > the labels will have a hard limit that can only be extended > > > > > > > with a major bump. This is what i implemented in this version. > > > > > > > - "char *name" per channel that the user may allocate and the > > > > > > > API will manage, duplicate and free. Simple solution, and the > > > > > > > name can be arbitrarily long, but inefficient (av_strdup() per > > > > > > > channel with a custom label on layout copy). > > > > > > > - "const char *name" per channel for compile time constants, or > > > > > > > that the user may allocate and free. Very efficient, but for > > > > > > > non compile time strings ensuring they outlive the layout can > > > > > > > be tricky. > > > > > > > - Refcounted AVChannelCustom with a dictionary. This can't be > > > > > > > done with AVBufferRef, so it would require some other form > > > > > > > of reference counting. And a dictionary may add quite a bit of > > > > > > > complexity to the API, as you can set anything on them. > > > > > > > > > > > > Until we have proper refcounting API we can make the AVBufferRef in > > > > > > AVChannelLayout a void *, and only allow channel_layout functions to > > > > > > dereference it as an AVBufferRef. This would mean adding some extra > > > > > > helper > > > > > > functions to channel layout, but overall it is not unsolvable. > > > > > > > > > > > > The real question is that if you want to use refcounting and add > > > > > > helpers to > > > > > > query / replace per-channel metadata, or you find the idea too > > > > > > heavy weight > > > > > > and would like to stick to flat structs. > > > > > > > > > > what is the advantage of refcounting for channel metadata ? > > > > > is it about the used memory, about the reduced need to copy ? > > > > > > > > Basicly it is the ability to store per-channel metadata in avdictionary, > > > > because otherwise it would have to be copyed, and avdictionary is very > > > > ineffective at copying because of many mallocs. > > > > > > > > > > > > > > what kind of metadata and what size do you expect ? > > > > > bytes, kilobytes, megabytes, gigabytes per channel ? > > > > > > > > Usually, nothing, because most format don't have support for per-channel > > > > metadata. In some cases it is going to be a couple of textual metadata > > > > key-value pairs, such as language, label, group, speaker, positon, so > > > > 4-5 > > > > dynamically allocated string pairs, plus the AVDictionary itself, > > > > multiplied > > > > by the number of channels in a layout. > > > > > > > > > > > > > > what is the overhead for dynamic allocation and ref counting? > > > > > that is at which point does it even make sense ? > > > > > > > > I don't have exact measurements. It is generally felt that copying > > > > AVDictionary per-channel is a huge overhead for something as > > > > lightweight as > > > > an audio frame which is a 2-4 kB per channel at most and only a couple > > > > of > > > > allocs usually not dependant on the number of channels. That's why > > > > refcounting was proposed. > > > > > > I was thinking more at a AVStream / AVCodecParameters level. > > > > > How will a demuxer transport such metadata over a AVPacket into a decoder > > > outputting metadata-filled AVFrames? > > > > or is this never needed ? > > I am not sure I understand. Usually metadata is passed from demuxer to > decoder by avcodec_parameters_to_context(), this is used for all metadata > which is in AVCodecParameters. > > For per-packet metadata ff_decode_frame_props() has some automatic packet > side data -> frame side data transfer. > > AVStream side data may be transferred to AVPacket side data if > av_format_inject_global_side_data() is used, but it is not enabled by > default. The sidedata in AVPacket is not channel specific, the data in AVFrames new channels is. The later is as you wrote expensive
Re: [FFmpeg-devel] Politics
On Sat, Dec 18, 2021 at 08:41:09PM +, Soft Works wrote: > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Daniel > > Cantarín > > Sent: Saturday, December 18, 2021 9:05 PM > > To: ffmpeg-devel@ffmpeg.org > > Subject: Re: [FFmpeg-devel] Politics > > > > > > > > Then you have never seen anime translations where signage in the > > > videos are translated. If the subtitles are off even by one frame in > > > such a case, you will see it, especially when the translated sign is > > > moving with the video, and one new subtitle event is generated for > > > every video frame. > > > You can't sync to audio when the element you are translating is in the > > > video itself, and not audio. > > > > > > - Hendrik > > > > This is correct, thank you. > > 1. If you're translating visuals, you sync to video. > > 2. If such visual is moving, you may move the translation in sync. > > > > I've ignored those cases, and it's correct to remark them. > > > > Yet, I understand this is done with video editing UIs, not ffmpeg filters, > > specially as it requires to visually match XY coordinates. > > > > Also, subtitle communities I know of use timings, not frames, even > > when doing overlays: "overlay at X:Y:Z.000 time". > > > > And a single frame means absolutelly nothing, even in this use cases. > > > > So, overall, I fail to see the serious frame-perfect subtitle-video sync > > problem with the patchset. > > The more the focus is moving to "a single frame" doesn't matter, > the more will that conversation create the impression that my patchset > would be lacking precision. > > In fact we're just talking about a fantasy subject instead of an > existing problem. > > > But there's no need to so much debate: get us some such anime sub, > > I get the original video somehow, do some tests, and post the results > > here. I'm open to do the testing work, if others are willing to help me > > with input examples. > > I would also be interested in an example for this, even when it doesn't > prove any issues. > > I'd be really glad when somebody could provide a sample (even privately), > it could be something I haven't seen yet. playing devils advocate here, meaning iam not sure if the example below makes sense as an argument in this debate but its a interresting case which was not mentioned consider a subtitle track consider 2 video tracks for US 3/1001 fps and EU 25 fps the 6th frame in the US track is at 0.2002 sec, the 5th in the EU track at 0.2sec if these differ and you want a subtitle to either stop displaying after the earlier or begin displaying after the earlier reliably. Then you need a timebase that can represent points within each such close encounter of frame times. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer. 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] qsvenc_hevc: Enable look ahead with ExtBRC
Xiang, Haihao 于2021年12月6日周一 11:09写道: > > --- a/libavcodec/qsvenc_hevc.c > > +++ b/libavcodec/qsvenc_hevc.c > > @@ -248,6 +248,7 @@ static const AVOption options[] = { > > { "tile_rows", "Number of rows for tiled > > encoding", OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 }, 0, > > UINT16_MAX, VE }, > > { "recovery_point_sei", "Insert recovery point SEI > > messages", OFFSET(qsv.recovery_point_sei), AV_OPT_TYPE_INT, { > > .i64 > > = -1 }, -1, 1, VE }, > > { "aud", "Insert the Access Unit Delimiter NAL", OFFSET(qsv.aud), > > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE}, > > +{ "look_ahead_depth", "Depth of look ahead in number frames", > > OFFSET(qsv.look_ahead_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 100, VE }, > > > > { NULL }, > > }; > > Any comment for this patch ? Could someone help to merge this patch if no > objection ? How to verify this function? is it depended on a special version of MSDK && vaapi-driver? I tried to enable extbrc and disable/enable look_ahead_depth (set to zero/20) for hevc_qsv but found there are no difference of the encoded files. ___ 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 04/10] lavc/videotoolboxenc: fix RGB support
On Thu, Dec 16, 2021 at 7:13 PM rcombs wrote: > --- > libavcodec/videotoolboxenc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index fa8f717a6c..e10373dded 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -1012,6 +1012,9 @@ static int get_cv_ycbcr_matrix(AVCodecContext > *avctx, CFStringRef *matrix) { > *matrix = compat_keys.kCVImageBufferYCbCrMatrix_ITU_R_2020; > break; > > +case AVCOL_SPC_RGB: > It would be clearer to set the matrix to NULL here (like the AVCOL_SPC_UNSPECIFIED case). +break; > + > default: > av_log(avctx, AV_LOG_ERROR, "Color space %s is not > supported.\n", av_color_space_name(avctx->colorspace)); > return -1; > -- > 2.33.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 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 06/10] lavc/videotoolboxenc: vastly simplify get_cv_pixel_info
On Thu, Dec 16, 2021 at 7:13 PM rcombs wrote: > No longer requires per-format switch cases. > > The frame==0 path was unused (and would've crashed anyway). > --- > libavcodec/videotoolboxenc.c | 69 +++- > 1 file changed, 13 insertions(+), 56 deletions(-) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index b66d44f6b7..82e01fbe29 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -2034,6 +2034,7 @@ static int get_cv_pixel_info( > size_t *strides, > size_t *contiguous_buf_size) > { > +const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt); > VTEncContext *vtctx = avctx->priv_data; > int av_format = frame->format; > int av_color_range = frame->color_range; > @@ -2041,6 +2042,9 @@ static int get_cv_pixel_info( > int range_guessed; > int status; > > +if (!desc) > +return AVERROR(EINVAL); > + > status = get_cv_pixel_format(avctx, av_format, av_color_range, color, > &range_guessed); > if (status) { > av_log(avctx, > @@ -2065,63 +2069,16 @@ static int get_cv_pixel_info( > } > } > > -switch (av_format) { > -case AV_PIX_FMT_NV12: > -*plane_count = 2; > - > -widths [0] = avctx->width; > -heights[0] = avctx->height; > -strides[0] = frame ? frame->linesize[0] : avctx->width; > - > -widths [1] = (avctx->width + 1) / 2; > -heights[1] = (avctx->height + 1) / 2; > -strides[1] = frame ? frame->linesize[1] : (avctx->width + 1) & -2; > -break; > - > -case AV_PIX_FMT_YUV420P: > -*plane_count = 3; > - > -widths [0] = avctx->width; > -heights[0] = avctx->height; > -strides[0] = frame ? frame->linesize[0] : avctx->width; > - > -widths [1] = (avctx->width + 1) / 2; > -heights[1] = (avctx->height + 1) / 2; > -strides[1] = frame ? frame->linesize[1] : (avctx->width + 1) / 2; > - > -widths [2] = (avctx->width + 1) / 2; > -heights[2] = (avctx->height + 1) / 2; > -strides[2] = frame ? frame->linesize[2] : (avctx->width + 1) / 2; > -break; > +*plane_count = av_pix_fmt_count_planes(avctx->pix_fmt); > > -case AV_PIX_FMT_BGRA: > -*plane_count = 1; > - > -widths [0] = avctx->width; > -heights[0] = avctx->height; > -strides[0] = frame ? frame->linesize[0] : avctx->width * 4; > -break; > - > -case AV_PIX_FMT_P010LE: > -*plane_count = 2; > -widths[0] = avctx->width; > -heights[0] = avctx->height; > -strides[0] = frame ? frame->linesize[0] : (avctx->width * 2 + 63) > & -64; > - > -widths[1] = (avctx->width + 1) / 2; > -heights[1] = (avctx->height + 1) / 2; > -strides[1] = frame ? frame->linesize[1] : ((avctx->width + 1) / 2 > + 63) & -64; > -break; > - > -default: > -av_log( > - avctx, > - AV_LOG_ERROR, > - "Could not get frame format info for color %d range %d.\n", > - av_format, > - av_color_range); > - > -return AVERROR(EINVAL); > +for (i = 0; i < desc->nb_components; i++) { > +int p = desc->comp[i].plane; > +int chroma = (p != 0) && (!(desc->flags & AV_PIX_FMT_FLAG_ALPHA) > || p + 1 != *plane_count); > This could be broken out into more variables to make the logic clearer. It took me a while to parse this. > +int shiftw = chroma ? desc->log2_chroma_w : 0; > +int shifth = chroma ? desc->log2_chroma_h : 0; > +widths[p] = (avctx->width + ((1 << shiftw) >> 1)) >> shiftw; > +heights[p] = (avctx->height + ((1 << shifth) >> 1)) >> shifth; > +strides[p] = frame->linesize[p]; > } > > *contiguous_buf_size = 0; > -- > 2.33.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 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] configure: fix metal detection and respect explicit disable
On Sat, Dec 18, 2021 at 9:35 PM Aman Karmani wrote: > From: Aman Karmani > > Signed-off-by: Aman Karmani > --- > configure | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/configure b/configure > index d8b07c8e00..d9d41b2273 100755 > --- a/configure > +++ b/configure > @@ -1758,6 +1758,7 @@ EXTERNAL_AUTODETECT_LIBRARY_LIST=" > libxcb_xfixes > lzma > mediafoundation > +metal > schannel > sdl2 > securetransport > @@ -6328,13 +6329,13 @@ enabled appkit && check_apple_framework > AppKit > enabled audiotoolbox && check_apple_framework AudioToolbox > enabled avfoundation && check_apple_framework AVFoundation > enabled coreimage&& check_apple_framework CoreImage > +enabled metal&& check_apple_framework Metal > enabled videotoolbox && check_apple_framework VideoToolbox > > check_apple_framework CoreFoundation > check_apple_framework CoreMedia > check_apple_framework CoreVideo > check_apple_framework CoreAudio > -check_apple_framework Metal > > enabled avfoundation && { > disable coregraphics applicationservices > -- > 2.33.0 Applied > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] lavc/videotoolboxenc: explicitly set realtime=false
On Sat, Dec 18, 2021 at 1:53 PM rcombs wrote: > On some encoders, this defaults to true, which can result in encode speed > being _limited_ to only slightly above realtime (as a power-saving > measure), > so we need a way to disable it. > --- > libavcodec/videotoolboxenc.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index 5f1e3a9b9c..3599d730d8 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -1391,10 +1391,10 @@ static int vtenc_create_encoder(AVCodecContext > *avctx, > } > } > > -if (vtctx->realtime) { > +if (vtctx->realtime >= 0) { > status = VTSessionSetProperty(vtctx->session, > > compat_keys.kVTCompressionPropertyKey_RealTime, > - kCFBooleanTrue); > + vtctx->realtime ? kCFBooleanTrue : > kCFBooleanFalse); LGTM > > if (status) { > av_log(avctx, AV_LOG_ERROR, "Error setting realtime property: > %d\n", status); > @@ -2676,7 +2676,7 @@ static const enum AVPixelFormat prores_pix_fmts[] = { > { "require_sw", "Require software encoding", OFFSET(require_sw), > AV_OPT_TYPE_BOOL, \ > { .i64 = 0 }, 0, 1, VE }, \ > { "realtime", "Hint that encoding should happen in real-time if not > faster (e.g. capturing from camera).", \ > -OFFSET(realtime), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE }, \ > +OFFSET(realtime), AV_OPT_TYPE_BOOL, { .i64 = 0 }, -1, 1, VE }, \ > { "frames_before", "Other frames will come before the frames in this > session. This helps smooth concatenation issues.", \ > OFFSET(frames_before), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE > }, \ > { "frames_after", "Other frames will come after the frames in this > session. This helps smooth concatenation issues.", \ > -- > 2.33.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 mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] libavcodec/videotoolboxenc: use the correct types for options
On Sat, Dec 18, 2021 at 1:53 PM rcombs wrote: > These are all set by AV_OPT_TYPE_INT or AV_OPT_TYPE_BOOL; the only reason > they worked before was that this is only used on little-endian. > --- > libavcodec/videotoolboxenc.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index 8237bd0ec5..5f1e3a9b9c 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -227,14 +227,14 @@ typedef struct VTEncContext { > int64_t dts_delta; > > int64_t profile; > -int64_t level; > -int64_t entropy; > -int64_t realtime; > -int64_t frames_before; > -int64_t frames_after; > - > -int64_t allow_sw; > -int64_t require_sw; > +int level; > +int entropy; > +int realtime; > +int frames_before; > +int frames_after; > + > +int allow_sw; > +int require_sw; LGTM > double alpha_quality; > > bool flushing; > -- > 2.33.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 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 07/10] lavc/videotoolboxenc: add handling for non-NAL-based codecs
On Thu, Dec 16, 2021 at 7:13 PM rcombs wrote: > --- > libavcodec/videotoolboxenc.c | 149 +-- > 1 file changed, 92 insertions(+), 57 deletions(-) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index 82e01fbe29..67bb563639 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -549,6 +549,7 @@ static int copy_param_sets( > > static int set_extradata(AVCodecContext *avctx, CMSampleBufferRef > sample_buffer) > { > +VTEncContext *vtctx = avctx->priv_data; > CMVideoFormatDescriptionRef vid_fmt; > size_t total_size; > int status; > @@ -559,23 +560,37 @@ static int set_extradata(AVCodecContext *avctx, > CMSampleBufferRef sample_buffer) > return AVERROR_EXTERNAL; > } > > -status = get_params_size(avctx, vid_fmt, &total_size); > -if (status) { > -av_log(avctx, AV_LOG_ERROR, "Could not get parameter sets.\n"); > -return status; > -} > +if (vtctx->get_param_set_func) { > +status = get_params_size(avctx, vid_fmt, &total_size); > +if (status) { > +av_log(avctx, AV_LOG_ERROR, "Could not get parameter > sets.\n"); > +return status; > +} > > -avctx->extradata = av_mallocz(total_size + > AV_INPUT_BUFFER_PADDING_SIZE); > -if (!avctx->extradata) { > -return AVERROR(ENOMEM); > -} > -avctx->extradata_size = total_size; > +avctx->extradata = av_mallocz(total_size + > AV_INPUT_BUFFER_PADDING_SIZE); > +if (!avctx->extradata) { > +return AVERROR(ENOMEM); > +} > +avctx->extradata_size = total_size; > > -status = copy_param_sets(avctx, vid_fmt, avctx->extradata, > total_size); > +status = copy_param_sets(avctx, vid_fmt, avctx->extradata, > total_size); > > -if (status) { > -av_log(avctx, AV_LOG_ERROR, "Could not copy param sets.\n"); > -return status; > +if (status) { > +av_log(avctx, AV_LOG_ERROR, "Could not copy param sets.\n"); > +return status; > +} > +} else { > +CFDataRef data = CMFormatDescriptionGetExtension(vid_fmt, > kCMFormatDescriptionExtension_VerbatimSampleDescription); > +if (data && CFGetTypeID(data) == CFDataGetTypeID()) { > +CFIndex size = CFDataGetLength(data); > + > +avctx->extradata = av_mallocz(size + > AV_INPUT_BUFFER_PADDING_SIZE); > +if (!avctx->extradata) > +return AVERROR(ENOMEM); > +avctx->extradata_size = size; > + > +CFDataGetBytes(data, CFRangeMake(0, size), avctx->extradata); > +} > } > > return 0; > @@ -1938,62 +1953,82 @@ static int vtenc_cm_to_avpacket( > CMTime dts; > CMVideoFormatDescriptionRef vid_fmt; > > - > vtenc_get_frame_info(sample_buffer, &is_key_frame); > -status = get_length_code_size(avctx, sample_buffer, > &length_code_size); > -if (status) return status; > > -add_header = is_key_frame && !(avctx->flags & > AV_CODEC_FLAG_GLOBAL_HEADER); > +if (vtctx->get_param_set_func) { > +status = get_length_code_size(avctx, sample_buffer, > &length_code_size); > +if (status) return status; > > -if (add_header) { > -vid_fmt = CMSampleBufferGetFormatDescription(sample_buffer); > -if (!vid_fmt) { > -av_log(avctx, AV_LOG_ERROR, "Cannot get format > description.\n"); > -return AVERROR_EXTERNAL; > +add_header = is_key_frame && !(avctx->flags & > AV_CODEC_FLAG_GLOBAL_HEADER); > + > +if (add_header) { > +vid_fmt = CMSampleBufferGetFormatDescription(sample_buffer); > +if (!vid_fmt) { > +av_log(avctx, AV_LOG_ERROR, "Cannot get format > description.\n"); > +return AVERROR_EXTERNAL; > +} > + > +int status = get_params_size(avctx, vid_fmt, &header_size); > +if (status) return status; > } > > -int status = get_params_size(avctx, vid_fmt, &header_size); > -if (status) return status; > -} > +status = count_nalus(length_code_size, sample_buffer, > &nalu_count); > +if(status) > +return status; > > -status = count_nalus(length_code_size, sample_buffer, &nalu_count); > -if(status) > -return status; > +if (sei) { > +size_t msg_size = get_sei_msg_bytes(sei, > + > SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35); > > -if (sei) { > -size_t msg_size = get_sei_msg_bytes(sei, > - > SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35); > +sei_nalu_size = sizeof(start_code) + 1 + msg_size + 1; > +} > > -sei_nalu_size = sizeof(start_code) + 1 + msg_size + 1; > -} > +in_buf_size = CMSampleBufferGetTotalSampleSize(sample_buffer); > +out_buf_size = header_size + > + in_buf_size + > + sei_nalu_size + >
Re: [FFmpeg-devel] [PATCH v4 3/5] avutil: add obj-c helpers into header-only include
On 12/17/2021 5:04 PM, Aman Karmani wrote: From: Aman Karmani Reviewed-by: Ridley Combs Signed-off-by: Aman Karmani --- libavutil/objc.h | 32 1 file changed, 32 insertions(+) create mode 100644 libavutil/objc.h diff --git a/libavutil/objc.h b/libavutil/objc.h new file mode 100644 index 00..0db993f716 --- /dev/null +++ b/libavutil/objc.h @@ -0,0 +1,32 @@ +/* + * 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 + */ + +#ifndef AVUTIL_OBJC_H +#define AVUTIL_OBJC_H + +#include + +static inline void ff_objc_release(NSObject **obj) +{ +if (*obj) { +[*obj release]; +*obj = nil; +} +} + +#endif /* AVUTIL_OBJC_H */ This breaks checkheaders. It should be added to SKIPHEADERS with the corresponding check. Also, why is this in lavu if it's ultimately only used in lavfi? And does it need to be a separate header at all? It's apparently not even 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".
Re: [FFmpeg-devel] lavc/videotoolboxenc: add ProRes support
On Thu, Dec 16, 2021 at 7:12 PM rcombs wrote: > This patchset includes several infrastructural improvements to > videotoolboxenc, > an added group of output formats for swscale, and a ProRes encoder wrapper. > > Also available as a GitHub PR here: > https://github.com/FFmpeg/FFmpeg/pull/378 > > Review comments and followup changes will be applied there to reduce ML > noise. > I noticed this a little late. I'll add my comments on github too. Is that the preferred way now? Seems better to me. > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/5] avutil: add obj-c helpers into header-only include
On Sun, Dec 19, 2021 at 9:09 AM James Almer wrote: > On 12/17/2021 5:04 PM, Aman Karmani wrote: > > From: Aman Karmani > > > > Reviewed-by: Ridley Combs > > Signed-off-by: Aman Karmani > > --- > > libavutil/objc.h | 32 > > 1 file changed, 32 insertions(+) > > create mode 100644 libavutil/objc.h > > > > diff --git a/libavutil/objc.h b/libavutil/objc.h > > new file mode 100644 > > index 00..0db993f716 > > --- /dev/null > > +++ b/libavutil/objc.h > > @@ -0,0 +1,32 @@ > > +/* > > + * 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 > > + */ > > + > > +#ifndef AVUTIL_OBJC_H > > +#define AVUTIL_OBJC_H > > + > > +#include > > + > > +static inline void ff_objc_release(NSObject **obj) > > +{ > > +if (*obj) { > > +[*obj release]; > > +*obj = nil; > > +} > > +} > > + > > +#endif /* AVUTIL_OBJC_H */ > > This breaks checkheaders. It should be added to SKIPHEADERS with the > corresponding check. > Sorry, will fix. > > Also, why is this in lavu if it's ultimately only used in lavfi? And > does it need to be a separate header at all? It's apparently not even C. > It is a generic helper requested by rcombs. I think the plan is to reuse it in other filters/decoders/encoders written in obj-c. One of the reasons is becausing calling `[nil release]` can crash. You're right that it is not C, its obj-c and that's why it was simpler to have it be an include only header. I'm not sure if using a different extension would have been preferred. Aman > ___ > 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] avutil: add objc.h to SKIPHEADERS
From: Aman Karmani Signed-off-by: Aman Karmani --- libavutil/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/libavutil/Makefile b/libavutil/Makefile index 529046dbc8..d17876df1a 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -195,6 +195,7 @@ OBJS += $(COMPAT_OBJS:%=../compat/%) # Windows resource file SLIBOBJS-$(HAVE_GNU_WINDRES)+= avutilres.o +SKIPHEADERS+= objc.h SKIPHEADERS-$(HAVE_CUDA_H) += hwcontext_cuda.h SKIPHEADERS-$(CONFIG_CUDA) += hwcontext_cuda_internal.h \ cuda_check.h -- 2.33.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 3/5] avutil: add obj-c helpers into header-only include
On 19 Dec 2021, at 18:35, Aman Karmani wrote: > On Sun, Dec 19, 2021 at 9:09 AM James Almer wrote: > >> On 12/17/2021 5:04 PM, Aman Karmani wrote: >>> From: Aman Karmani >>> >>> Reviewed-by: Ridley Combs >>> Signed-off-by: Aman Karmani >>> --- >>> libavutil/objc.h | 32 >>> 1 file changed, 32 insertions(+) >>> create mode 100644 libavutil/objc.h >>> >>> diff --git a/libavutil/objc.h b/libavutil/objc.h >>> new file mode 100644 >>> index 00..0db993f716 >>> --- /dev/null >>> +++ b/libavutil/objc.h >>> @@ -0,0 +1,32 @@ >>> +/* >>> + * 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 >>> + */ >>> + >>> +#ifndef AVUTIL_OBJC_H >>> +#define AVUTIL_OBJC_H >>> + >>> +#include >>> + >>> +static inline void ff_objc_release(NSObject **obj) >>> +{ >>> +if (*obj) { >>> +[*obj release]; >>> +*obj = nil; >>> +} >>> +} >>> + >>> +#endif /* AVUTIL_OBJC_H */ >> >> This breaks checkheaders. It should be added to SKIPHEADERS with the >> corresponding check. >> > > Sorry, will fix. > > >> >> Also, why is this in lavu if it's ultimately only used in lavfi? And >> does it need to be a separate header at all? It's apparently not even C. >> > > It is a generic helper requested by rcombs. I think the plan is to reuse it > in other filters/decoders/encoders written in obj-c. > > One of the reasons is becausing calling `[nil release]` can crash. That's not true, dispatching a message to nil in Objective-C has no effect. I don't really see the benefit of this helper, personally. > > You're right that it is not C, its obj-c and that's why it was simpler to > have it be an include only header. I'm not sure if using a different > extension would have been preferred. > > Aman > > >> ___ >> 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 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] Politics
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Sunday, December 19, 2021 4:16 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] Politics > > On Sat, Dec 18, 2021 at 08:41:09PM +, Soft Works wrote: > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Daniel > > > Cantarín > > > Sent: Saturday, December 18, 2021 9:05 PM > > > To: ffmpeg-devel@ffmpeg.org > > > Subject: Re: [FFmpeg-devel] Politics > > > > > > > > > > > Then you have never seen anime translations where signage in the > > > > videos are translated. If the subtitles are off even by one frame in > > > > such a case, you will see it, especially when the translated sign is > > > > moving with the video, and one new subtitle event is generated for > > > > every video frame. > > > > You can't sync to audio when the element you are translating is in the > > > > video itself, and not audio. > > > > > > > > - Hendrik > > > > > > This is correct, thank you. > > > 1. If you're translating visuals, you sync to video. > > > 2. If such visual is moving, you may move the translation in sync. > > > > > > I've ignored those cases, and it's correct to remark them. > > > > > > Yet, I understand this is done with video editing UIs, not ffmpeg > filters, > > > specially as it requires to visually match XY coordinates. > > > > > > Also, subtitle communities I know of use timings, not frames, even > > > when doing overlays: "overlay at X:Y:Z.000 time". > > > > > > And a single frame means absolutelly nothing, even in this use cases. > > > > > > So, overall, I fail to see the serious frame-perfect subtitle-video sync > > > problem with the patchset. > > > > The more the focus is moving to "a single frame" doesn't matter, > > the more will that conversation create the impression that my patchset > > would be lacking precision. > > > > In fact we're just talking about a fantasy subject instead of an > > existing problem. > > > > > But there's no need to so much debate: get us some such anime sub, > > > I get the original video somehow, do some tests, and post the results > > > here. I'm open to do the testing work, if others are willing to help me > > > with input examples. > > > > I would also be interested in an example for this, even when it doesn't > > prove any issues. > > > > I'd be really glad when somebody could provide a sample (even privately), > > it could be something I haven't seen yet. > > playing devils advocate here, meaning iam not sure if the example below makes > sense as an argument in this debate but its a interresting case which was > not mentioned > > consider a subtitle track > consider 2 video tracks for US 3/1001 fps and EU 25 fps > > the 6th frame in the US track is at 0.2002 sec, the 5th in the EU track at > 0.2sec Yes, and we urgently need to buy carrots, because tomorrow, a unicorn might come around the corner and then we wouldn't have anything to feed it. Also, the whole MPEG code should be re-written because next year, there could be a breaking change to the specs, so we should go and rewrite the code to prepare for that possibility. Also we should all buy a red T-Shirt, because aliens might arrive soon at our planet, and they will probably like red colors. ;-) At the serious side: First of all, if such a unicorn video would exist, and you would want to consider this as a realistic problem, then it would be a problem of its own and independent of ffmpeg and there's nothing that ffmpeg could remedy. It's the time resolution of all the subtitle format specifications which do not offer such a fine granularity. And this won't change. Simply because it's not needed and there wouldn't be the slightest advantage. The idea that a high resolution for subtitle timings would be required for being able to match video timings as closely as possible is a misconception. In fact, you couldn't do more wrong than that. :-) (I'm sure some will figure out why) For the unicorn video, we'd first need to know how it was produced. From a movie? Then the two tracks would have a different playback speed, and the two video couldn't use the same subtitle track anyway. I don't think that a camera exists that has a framerate high enough so that you could produce videos at both framerates just from picking original frames. In most other cases, one of the videos would have been converted from the other one. There's high-priced studio and broadcast equipment for doing such conversions, and it requires a mix of different strategies for this. But these can't do magic either, and when we go back to the example above, we'd have to start by wondering what is happening with hard cuts when such conversions are done. Either a hard cut would turn into a slightly smoothed transition over 2 or 3 frames, then subs wouldn't be affected (still hitting the smooth transition. On the other hand, when you would do hard cuts at the
Re: [FFmpeg-devel] Politics
> -Original Message- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Sunday, December 19, 2021 7:23 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] Politics > > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Michael > > Niedermayer > > Sent: Sunday, December 19, 2021 4:16 PM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] Politics > > > > On Sat, Dec 1 8, 2021 at 08:41:09PM +, Soft Works wrote: > I don't think that a camera exists that has a framerate high enough so that > you could produce videos at both framerates just from picking original > frames. I forgot to mention that I mean a high speed camera which can work at such an odd rate (least common multiple..) ___ 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] checkasm: store and associate contexts with functions and use it for av_tx
checkasm: store and associate contexts with functions and use it for av_tx The issue is the following: - checkasm/av_tx.c initializes a context and a function - check_func() receives the function only and returns NULL - checkasm/av_tx.c runs again with a different CPU flag and creates a new function and context - check_func() sees a new function for testing and sets func_ref to the function it first received - call_ref() and call_new() get called with the same, new context This means that av_tx contexts had to be compatible with both C and assembly functions. And so each context difference had to be generated twice and duplicated to keep checkasm working. The commit introduces a new *cont in the checkasm function struct, which is associated with each function. Code that doesn't need an additional context requires no changes, as we use wrapper macros. A downside is that there's no way to free contexts until the very end. However, as checkasm is a testing infrastructure, I think it's reasonable, and we're talking a few tens of kilobytes of heap memory as we only test a limited number of smaller transforms. Patch attached. >From 65d655740d75918dc78ba562a6ad682bfa55f480 Mon Sep 17 00:00:00 2001 From: Lynne Date: Sun, 19 Dec 2021 19:44:40 +0100 Subject: [PATCH] checkasm: store and associate contexts with functions and use it for av_tx The issue is the following: - checkasm/av_tx.c initializes a context and a function - check_func() receives the function only and returns NULL - checkasm/av_tx.c runs again with a different CPU flag and creates a new function and context - check_func() sees a new function for testing and sets func_ref to the function it first received - call_ref() and call_new() get called with the same, new context This means that av_tx contexts had to be compatible with both C and assembly functions. And so each context difference had to be generated twice and duplicated to keep checkasm working. The commit introduces a new *cont in the checkasm function struct, which is associated with each function. Code that doesn't need an additional context requires no changes, as we use wrapper macros. A downside is that there's no way to free contexts until the very end. However, as checkasm is a testing infrastructure, I think it's reasonable, and we're talking a few tens of kilobytes of heap memory as we only test a limited number of smaller transforms. --- tests/checkasm/av_tx.c| 10 +- tests/checkasm/checkasm.c | 21 - tests/checkasm/checkasm.h | 10 +++--- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/tests/checkasm/av_tx.c b/tests/checkasm/av_tx.c index 178fb61972..afa5dbac72 100644 --- a/tests/checkasm/av_tx.c +++ b/tests/checkasm/av_tx.c @@ -58,11 +58,11 @@ static const int check_lens[] = { return; \ } \ \ -if (check_func(fn, PREFIX "_%i", len)) { \ +if (check_func_cont(fn, tx, PREFIX "_%i", len)) { \ num_checks++; \ last_check = len; \ -call_ref(tx, out_ref, in, sizeof(DATA_TYPE)); \ -call_new(tx, out_new, in, sizeof(DATA_TYPE)); \ +call_ref(cont_ref, out_ref, in, sizeof(DATA_TYPE)); \ +call_new(cont_new, out_new, in, sizeof(DATA_TYPE)); \ if (CHECK_EXPRESSION) { \ fail(); \ break;\ @@ -70,11 +70,11 @@ static const int check_lens[] = { bench_new(tx, out_new, in, sizeof(DATA_TYPE));\ } \ \ -av_tx_uninit(&tx);\ +tx = NULL;\ fn = NULL;\ } \ \ -av_tx_uninit(&tx);\ +tx = NULL;\ fn = NULL;
[FFmpeg-devel] [PATCH v5 00/13] dshow enhancements
This series solves some outstanding bugs in the dshow device, implements get_device_list so that `ffmpeg -sources dshow` works and adds logic to select a video format with extended color information (color range, space, etc) if exposed by the device. v5 adresses a comment received offlist for patch 5/13 (now emits log message when a sample timestamp is missing. This is a new version of part of the patch series in https://ffmpeg.org/pipermail/ffmpeg-devel/2021-July/282073.html, addressing review comments i got there. They have been previously OK'ed offlist by the dshow maintainer Roger Pack. Specifically, these patches are the enhancements that should be uncontroversial as they do not touch the avformat API. I hope they can be swiftly pushed by someone, then i'll re-send the other more controversial patches on top of these. Diederick Niehorster (13): avdevice/dshow: prevent NULL access avdevice/dshow: implement option to use device video timestamps avdevice/dshow: add use_video_device_timestamps to docs avdevice/dshow: query graph and sample time only once avdevice/dshow: handle unknown sample time avdevice/dshow: set no-seek flags avdevice/dshow: implement get_device_list avdevice/dshow: list_devices: show media type(s) per device avdevice: add info about media types(s) to AVDeviceInfo avdevice/dshow: add media type info to get_device_list fftools: provide media type info for devices avdevice/dshow: discover source color range/space/etc avdevice/dshow: select format with extended color info doc/indevs.texi | 6 + fftools/cmdutils.c | 34 +- libavdevice/avdevice.c | 2 + libavdevice/avdevice.h | 2 + libavdevice/dshow.c | 838 +++- libavdevice/dshow_capture.h | 1 + libavdevice/dshow_pin.c | 52 ++- libavdevice/version.h | 2 +- 8 files changed, 801 insertions(+), 136 deletions(-) -- 2.28.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 v5 01/13] avdevice/dshow: prevent NULL access
list_options true would crash when both a video and an audio device were specified as input. Crash would occur on line 784 because ctx->device_unique_name[otherDevType] would be NULL Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index ef78781865..cc0bef0474 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -708,9 +708,9 @@ dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum, if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name)) < 0) return r; ctx->device_filter[devtype] = device_filter; +ctx->device_unique_name[devtype] = device_unique_name; if ((r = dshow_cycle_pins(avctx, devtype, sourcetype, device_filter, NULL)) < 0) return r; -av_freep(&device_unique_name); return 0; } @@ -1143,6 +1143,7 @@ static int dshow_read_header(AVFormatContext *avctx) } } } +// don't exit yet, allow it to list crossbar options in dshow_open_device } if (ctx->device_name[VideoDevice]) { if ((r = dshow_open_device(avctx, devenum, VideoDevice, VideoSourceDevice)) < 0 || -- 2.28.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 v5 02/13] avdevice/dshow: implement option to use device video timestamps
The dshow avdevice ignores timestamps for video frames provided by the DirectShow device, instead using wallclock time, apparently because the implementer of this code had a device that provided unreliable timestamps. Me (and others) would like to use the device's timestamps. The new use_video_device_timestamps option for dshow device enables them to do so. Since the majority of video devices out there probably provide fine timestamps, this patch sets the default to using the device timestamps, which means best fidelity timestamps are used by default. Using the new option, the user can switch this off and revert to the old behavior, so a fall back remains available in case the device provides broken timestamps. Closes: #8620 Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 1 + libavdevice/dshow_capture.h | 1 + libavdevice/dshow_pin.c | 11 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index cc0bef0474..5e6eb9c85d 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -1310,6 +1310,7 @@ static const AVOption options[] = { { "audio_device_save", "save audio capture filter device (and properties) to file", OFFSET(audio_filter_save_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC }, { "video_device_load", "load video capture filter device (and properties) from file", OFFSET(video_filter_load_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC }, { "video_device_save", "save video capture filter device (and properties) to file", OFFSET(video_filter_save_file), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, DEC }, +{ "use_video_device_timestamps", "use device instead of wallclock timestamps for video frames", OFFSET(use_video_device_timestamps), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, DEC }, { NULL }, }; diff --git a/libavdevice/dshow_capture.h b/libavdevice/dshow_capture.h index 06ded2ba96..5a2691518c 100644 --- a/libavdevice/dshow_capture.h +++ b/libavdevice/dshow_capture.h @@ -312,6 +312,7 @@ struct dshow_ctx { char *audio_filter_save_file; char *video_filter_load_file; char *video_filter_save_file; +int use_video_device_timestamps; IBaseFilter *device_filter[2]; IPin*device_pin[2]; diff --git a/libavdevice/dshow_pin.c b/libavdevice/dshow_pin.c index 3dae405e65..8e56dccbfe 100644 --- a/libavdevice/dshow_pin.c +++ b/libavdevice/dshow_pin.c @@ -309,10 +309,14 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) if (!sample) return E_POINTER; +priv_data = pin->filter->priv_data; +s = priv_data; +ctx = s->priv_data; + IMediaSample_GetTime(sample, &orig_curtime, &dummy); orig_curtime += pin->filter->start_time; IReferenceClock_GetTime(clock, &graphtime); -if (devtype == VideoDevice) { +if (devtype == VideoDevice && !ctx->use_video_device_timestamps) { /* PTS from video devices is unreliable. */ IReferenceClock_GetTime(clock, &curtime); } else { @@ -322,7 +326,7 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) like 437650244077016960 which FFmpeg doesn't like. TODO figure out math. For now just drop them. */ av_log(NULL, AV_LOG_DEBUG, -"dshow dropping initial (or ending) audio frame with odd PTS too high %"PRId64"\n", curtime); +"dshow dropping initial (or ending) frame with odd PTS too high %"PRId64"\n", curtime); return S_OK; } curtime += pin->filter->start_time; @@ -330,9 +334,6 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) buf_size = IMediaSample_GetActualDataLength(sample); IMediaSample_GetPointer(sample, &buf); -priv_data = pin->filter->priv_data; -s = priv_data; -ctx = s->priv_data; index = pin->filter->stream_index; av_log(NULL, AV_LOG_VERBOSE, "dshow passing through packet of type %s size %8d " -- 2.28.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 v5 03/13] avdevice/dshow: add use_video_device_timestamps to docs
Signed-off-by: Diederick Niehorster --- doc/indevs.texi | 6 ++ 1 file changed, 6 insertions(+) diff --git a/doc/indevs.texi b/doc/indevs.texi index 5be647f70a..9d8020311a 100644 --- a/doc/indevs.texi +++ b/doc/indevs.texi @@ -625,6 +625,12 @@ Save the currently used video capture filter device and its parameters (if the filter supports it) to a file. If a file with the same name exists it will be overwritten. +@item use_video_device_timestamps +If set to @option{false}, the timestamp for video frames will be +derived from the wallclock instead of the timestamp provided by +the capture device. This allows working around devices that +provide unreliable timestamps. + @end table @subsection Examples -- 2.28.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 v5 04/13] avdevice/dshow: query graph and sample time only once
No need to query twice, use value we've already unconditionally got. Improve variable names Signed-off-by: Diederick Niehorster --- libavdevice/dshow_pin.c | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/libavdevice/dshow_pin.c b/libavdevice/dshow_pin.c index 8e56dccbfe..1d0e880480 100644 --- a/libavdevice/dshow_pin.c +++ b/libavdevice/dshow_pin.c @@ -295,9 +295,10 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) uint8_t *buf; int buf_size; /* todo should be a long? */ int index; -int64_t curtime; -int64_t orig_curtime; +int64_t chosentime; +int64_t sampletime; int64_t graphtime; +int use_sample_time = 1; const char *devtypename = (devtype == VideoDevice) ? "video" : "audio"; IReferenceClock *clock = pin->filter->clock; int64_t dummy; @@ -313,24 +314,27 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) s = priv_data; ctx = s->priv_data; -IMediaSample_GetTime(sample, &orig_curtime, &dummy); -orig_curtime += pin->filter->start_time; +IMediaSample_GetTime(sample, &sampletime, &dummy); IReferenceClock_GetTime(clock, &graphtime); if (devtype == VideoDevice && !ctx->use_video_device_timestamps) { /* PTS from video devices is unreliable. */ -IReferenceClock_GetTime(clock, &curtime); +chosentime = graphtime; +use_sample_time = 0; } else { -IMediaSample_GetTime(sample, &curtime, &dummy); -if(curtime > 40LL) { +if (sampletime > 40LL) { /* initial frames sometimes start < 0 (shown as a very large number here, - like 437650244077016960 which FFmpeg doesn't like. + like 437650244077016960 which FFmpeg doesn't like). TODO figure out math. For now just drop them. */ av_log(NULL, AV_LOG_DEBUG, -"dshow dropping initial (or ending) frame with odd PTS too high %"PRId64"\n", curtime); +"dshow dropping initial (or ending) frame with odd PTS too high %"PRId64"\n", sampletime); return S_OK; } -curtime += pin->filter->start_time; +chosentime = sampletime; } +// media sample time is relative to graph start time +sampletime += pin->filter->start_time; +if (use_sample_time) +chosentime += pin->filter->start_time; buf_size = IMediaSample_GetActualDataLength(sample); IMediaSample_GetPointer(sample, &buf); @@ -338,8 +342,8 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) av_log(NULL, AV_LOG_VERBOSE, "dshow passing through packet of type %s size %8d " "timestamp %"PRId64" orig timestamp %"PRId64" graph timestamp %"PRId64" diff %"PRId64" %s\n", -devtypename, buf_size, curtime, orig_curtime, graphtime, graphtime - orig_curtime, ctx->device_name[devtype]); -pin->filter->callback(priv_data, index, buf, buf_size, curtime, devtype); +devtypename, buf_size, chosentime, sampletime, graphtime, graphtime - sampletime, ctx->device_name[devtype]); +pin->filter->callback(priv_data, index, buf, buf_size, chosentime, devtype); return S_OK; } -- 2.28.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 v5 05/13] avdevice/dshow: handle unknown sample time
GetTime may return an error indication that the sample has not timestamps, or may return a NULL start time. In those cases, fall back to graph time. Emit log when that happens. Improve logging in the frame receive function: now logged against correct avclass instead of NULL. Better debug message in case sample dropped: could now be audio or video frame. Signed-off-by: Diederick Niehorster --- libavdevice/dshow_pin.c | 27 +-- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/libavdevice/dshow_pin.c b/libavdevice/dshow_pin.c index 1d0e880480..2d1fa0e882 100644 --- a/libavdevice/dshow_pin.c +++ b/libavdevice/dshow_pin.c @@ -295,14 +295,15 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) uint8_t *buf; int buf_size; /* todo should be a long? */ int index; -int64_t chosentime; -int64_t sampletime; -int64_t graphtime; +int64_t chosentime = 0; +int64_t sampletime = 0; +int64_t graphtime = 0; int use_sample_time = 1; const char *devtypename = (devtype == VideoDevice) ? "video" : "audio"; IReferenceClock *clock = pin->filter->clock; int64_t dummy; struct dshow_ctx *ctx; +HRESULT hr; dshowdebug("ff_dshow_meminputpin_Receive(%p)\n", this); @@ -314,22 +315,28 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) s = priv_data; ctx = s->priv_data; -IMediaSample_GetTime(sample, &sampletime, &dummy); +hr = IMediaSample_GetTime(sample, &sampletime, &dummy); IReferenceClock_GetTime(clock, &graphtime); if (devtype == VideoDevice && !ctx->use_video_device_timestamps) { /* PTS from video devices is unreliable. */ chosentime = graphtime; use_sample_time = 0; } else { -if (sampletime > 40LL) { +if (hr == VFW_E_SAMPLE_TIME_NOT_SET || sampletime == 0) { +chosentime = graphtime; +use_sample_time = 0; +av_log(s, AV_LOG_DEBUG, +"frame with missing sample timestamp encountered, falling back to graph timestamp\n"); +} +else if (sampletime > 40LL) { /* initial frames sometimes start < 0 (shown as a very large number here, like 437650244077016960 which FFmpeg doesn't like). TODO figure out math. For now just drop them. */ -av_log(NULL, AV_LOG_DEBUG, -"dshow dropping initial (or ending) frame with odd PTS too high %"PRId64"\n", sampletime); +av_log(s, AV_LOG_DEBUG, +"dropping initial (or ending) sample with odd PTS too high %"PRId64"\n", sampletime); return S_OK; -} -chosentime = sampletime; +} else +chosentime = sampletime; } // media sample time is relative to graph start time sampletime += pin->filter->start_time; @@ -340,7 +347,7 @@ long ff_dshow_meminputpin_Receive(DShowMemInputPin *this, IMediaSample *sample) IMediaSample_GetPointer(sample, &buf); index = pin->filter->stream_index; -av_log(NULL, AV_LOG_VERBOSE, "dshow passing through packet of type %s size %8d " +av_log(s, AV_LOG_VERBOSE, "passing through packet of type %s size %8d " "timestamp %"PRId64" orig timestamp %"PRId64" graph timestamp %"PRId64" diff %"PRId64" %s\n", devtypename, buf_size, chosentime, sampletime, graphtime, graphtime - sampletime, ctx->device_name[devtype]); pin->filter->callback(priv_data, index, buf, buf_size, chosentime, devtype); -- 2.28.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 v5 06/13] avdevice/dshow: set no-seek flags
avdevice/dshow is a realtime device and as such does not support seeking. Therefore, its demuxer format should define the AVFMT_NOBINSEARCH, AVFMT_NOGENSEARCH and AVFMT_NO_BYTE_SEEK flags. With these flags set, attempting to seek (with, e.g., avformat_seek_file()) correctly yields -1 (operation not permitted) instead of -22 (invalid argument). This actually seems to apply to many other devices, at least the gdigrab, v4l2, vfwcap, x11grab, fbdev, kmsgrab and android_camera devices, from reading the source. Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index 5e6eb9c85d..0ef3b3d13e 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -1329,6 +1329,6 @@ const AVInputFormat ff_dshow_demuxer = { .read_header= dshow_read_header, .read_packet= dshow_read_packet, .read_close = dshow_read_close, -.flags = AVFMT_NOFILE, +.flags = AVFMT_NOFILE | AVFMT_NOBINSEARCH | AVFMT_NOGENSEARCH | AVFMT_NO_BYTE_SEEK, .priv_class = &dshow_class, }; -- 2.28.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 v5 07/13] avdevice/dshow: implement get_device_list
Needed to enable programmatic discovery of DirectShow devices Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 80 + 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index 0ef3b3d13e..8c257ca8e7 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -202,11 +202,14 @@ fail: * retrieve the device with type specified by devtype and return the * pointer to the object found in *pfilter. * If pfilter is NULL, list all device names. + * If device_list is not NULL, populate it with found devices instead of + * outputting device names to log */ static int dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, enum dshowDeviceType devtype, enum dshowSourceFilterType sourcetype, -IBaseFilter **pfilter, char **device_unique_name) +IBaseFilter **pfilter, char **device_unique_name, +AVDeviceInfoList **device_list) { struct dshow_ctx *ctx = avctx->priv_data; IBaseFilter *device_filter = NULL; @@ -238,6 +241,7 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, IBindCtx *bind_ctx = NULL; LPOLESTR olestr = NULL; LPMALLOC co_malloc = NULL; +AVDeviceInfo *device = NULL; int i; r = CoGetMalloc(1, &co_malloc); @@ -282,11 +286,39 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, // success, loop will end now } } else { -av_log(avctx, AV_LOG_INFO, " \"%s\"\n", friendly_name); -av_log(avctx, AV_LOG_INFO, "Alternative name \"%s\"\n", unique_name); +if (device_list) { +device = av_mallocz(sizeof(AVDeviceInfo)); +if (!device) +goto fail1; + +device->device_name = av_strdup(friendly_name); +device->device_description = av_strdup(unique_name); +if (!device->device_name || !device->device_description) +goto fail1; + +// store to device_list output +if (av_reallocp_array(&(*device_list)->devices, + (*device_list)->nb_devices + 1, + sizeof(*(*device_list)->devices)) < 0) +goto fail1; +(*device_list)->devices[(*device_list)->nb_devices] = device; +(*device_list)->nb_devices++; +device = NULL; // copied into array, make sure not freed below +} +else { +av_log(avctx, AV_LOG_INFO, " \"%s\"\n", friendly_name); +av_log(avctx, AV_LOG_INFO, "Alternative name \"%s\"\n", unique_name); +} } fail1: +if (device) { +if (device->device_name) +av_freep(&device->device_name); +if (device->device_name) +av_freep(&device->device_description); +av_free(device); +} if (olestr && co_malloc) IMalloc_Free(co_malloc, olestr); if (bind_ctx) @@ -312,6 +344,39 @@ fail1: return 0; } +static int dshow_get_device_list(AVFormatContext *avctx, AVDeviceInfoList *device_list) +{ +struct dshow_ctx *ctx = avctx->priv_data; +ICreateDevEnum *devenum = NULL; +int r; +int ret = AVERROR(EIO); + +if (!device_list) +return AVERROR(EINVAL); + +CoInitialize(0); + +r = CoCreateInstance(&CLSID_SystemDeviceEnum, NULL, CLSCTX_INPROC_SERVER, +&IID_ICreateDevEnum, (void**)&devenum); +if (r != S_OK) { +av_log(avctx, AV_LOG_ERROR, "Could not enumerate system devices.\n"); +goto error; +} + +ret = dshow_cycle_devices(avctx, devenum, VideoDevice, VideoSourceDevice, NULL, NULL, &device_list); +if (ret < S_OK) +goto error; +ret = dshow_cycle_devices(avctx, devenum, AudioDevice, AudioSourceDevice, NULL, NULL, &device_list); + +error: +if (devenum) +ICreateDevEnum_Release(devenum); + +CoUninitialize(); + +return ret; +} + /** * Cycle through available formats using the specified pin, * try to set parameters specified through AVOptions and if successful @@ -705,7 +770,7 @@ dshow_list_device_options(AVFormatContext *avctx, ICreateDevEnum *devenum, char *device_unique_name = NULL; int r; -if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name)) < 0) +if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, &device_filter, &device_unique_name, NULL)) < 0) return r; ctx->device_filter[devtype] = device_filter; ctx->device_unique_name[devtype] = device_unique_name; @@ -765,7 +830,7 @@ dshow_open_device(AVFormatContext *avctx, ICreateDevEnum *devenum, av_log(avctx, AV_LOG_INFO, "C
[FFmpeg-devel] [PATCH v5 08/13] avdevice/dshow: list_devices: show media type(s) per device
the list_devices option of dshow didn't indicate whether a specific device provides audio or video output. This patch iterates through all media formats of all pins exposed by the device to see what types it provides for capture, and prints this to the console for each device. Importantly, this now allows to find devices that provide both audio and video, and devices that provide neither. Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 103 +--- 1 file changed, 98 insertions(+), 5 deletions(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index 8c257ca8e7..f25537db5c 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -197,6 +197,79 @@ fail: return; } +static void dshow_get_device_media_types(AVFormatContext *avctx, enum dshowDeviceType devtype, + enum dshowSourceFilterType sourcetype, IBaseFilter *device_filter, + enum AVMediaType **media_types, int *nb_media_types) +{ +struct dshow_ctx *ctx = avctx->priv_data; +IEnumPins *pins = 0; +IPin *pin; +int has_audio = 0, has_video = 0; + +if (IBaseFilter_EnumPins(device_filter, &pins) != S_OK) +return; + +while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK) { +IKsPropertySet *p = NULL; +PIN_INFO info = { 0 }; +GUID category; +DWORD r2; +IEnumMediaTypes *types = NULL; +AM_MEDIA_TYPE *type; + +if (IPin_QueryPinInfo(pin, &info) != S_OK) +goto next; +IBaseFilter_Release(info.pFilter); + +if (info.dir != PINDIR_OUTPUT) +goto next; +if (IPin_QueryInterface(pin, &IID_IKsPropertySet, (void **) &p) != S_OK) +goto next; +if (IKsPropertySet_Get(p, &ROPSETID_Pin, AMPROPERTY_PIN_CATEGORY, + NULL, 0, &category, sizeof(GUID), &r2) != S_OK) +goto next; +if (!IsEqualGUID(&category, &PIN_CATEGORY_CAPTURE)) +goto next; + +if (IPin_EnumMediaTypes(pin, &types) != S_OK) +goto next; + +// enumerate media types exposed by pin +// NB: don't know if a pin can expose both audio and video, check 'm all to be safe +IEnumMediaTypes_Reset(types); +while (IEnumMediaTypes_Next(types, 1, &type, NULL) == S_OK) { +if (IsEqualGUID(&type->majortype, &MEDIATYPE_Video)) { +has_video = 1; +} else if (IsEqualGUID(&type->majortype, &MEDIATYPE_Audio)) { +has_audio = 1; +} +CoTaskMemFree(type); +} + +next: +if (types) +IEnumMediaTypes_Release(types); +if (p) +IKsPropertySet_Release(p); +if (pin) +IPin_Release(pin); +} + +IEnumPins_Release(pins); + +if (has_audio || has_video) { +int nb_types = has_audio + has_video; +*media_types = av_malloc_array(nb_types, sizeof(enum AVMediaType)); +if (*media_types) { +if (has_audio) +(*media_types)[0] = AVMEDIA_TYPE_AUDIO; +if (has_video) +(*media_types)[0 + has_audio] = AVMEDIA_TYPE_VIDEO; +*nb_media_types = nb_types; +} +} +} + /** * Cycle through available devices using the device enumerator devenum, * retrieve the device with type specified by devtype and return the @@ -242,6 +315,8 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, LPOLESTR olestr = NULL; LPMALLOC co_malloc = NULL; AVDeviceInfo *device = NULL; +enum AVMediaType *media_types = NULL; +int nb_media_types = 0; int i; r = CoGetMalloc(1, &co_malloc); @@ -286,6 +361,12 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, // success, loop will end now } } else { +// get media types exposed by pins of device +if (IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void* ) &device_filter) == S_OK) { +dshow_get_device_media_types(avctx, devtype, sourcetype, device_filter, &media_types, &nb_media_types); +IBaseFilter_Release(device_filter); +device_filter = NULL; +} if (device_list) { device = av_mallocz(sizeof(AVDeviceInfo)); if (!device) @@ -306,12 +387,26 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, device = NULL; // copied into array, make sure not freed below } else { -av_log(avctx, AV_LOG_INFO, " \"%s\"\n", friendly_name); -av_log(avctx, AV_LOG_INFO, "Alternative name \"%s\"\n", unique_name); +av_log(avctx, AV_LOG_INFO, "\"%s\"", friendly_name); +if (nb_media_types > 0 && media_types) { +
[FFmpeg-devel] [PATCH v5 09/13] avdevice: add info about media types(s) to AVDeviceInfo
An avdevice, regardless of whether its category says its an audio or video device, may provide access to devices providing different media types, or even single devices providing multiple media types. Also, some devices may provide no media types. dshow is an example encompassing all of these cases. Users should be provided with this information, so AVDeviceInfo is extended to provide it. Bump avdevice version Signed-off-by: Diederick Niehorster --- libavdevice/avdevice.c | 2 ++ libavdevice/avdevice.h | 2 ++ libavdevice/version.h | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c index 2ae26ab8e3..712ef1e80c 100644 --- a/libavdevice/avdevice.c +++ b/libavdevice/avdevice.c @@ -157,6 +157,8 @@ void avdevice_free_list_devices(AVDeviceInfoList **device_list) if (dev) { av_freep(&dev->device_name); av_freep(&dev->device_description); +if (dev->media_types) +av_freep(&dev->media_types); av_free(dev); } } diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h index 8370bbc7f2..6f24976dcc 100644 --- a/libavdevice/avdevice.h +++ b/libavdevice/avdevice.h @@ -457,6 +457,8 @@ void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContex typedef struct AVDeviceInfo { char *device_name; /**< device name, format depends on device */ char *device_description;/**< human friendly name */ +enum AVMediaType *media_types; /**< array indicating what media types(s), if any, a device can provide. If null, cannot provide any */ +int nb_media_types; /**< length of media_types array, 0 if device cannot provide any media types */ } AVDeviceInfo; /** diff --git a/libavdevice/version.h b/libavdevice/version.h index 914e156ec7..c549768e12 100644 --- a/libavdevice/version.h +++ b/libavdevice/version.h @@ -28,7 +28,7 @@ #include "libavutil/version.h" #define LIBAVDEVICE_VERSION_MAJOR 59 -#define LIBAVDEVICE_VERSION_MINOR 0 +#define LIBAVDEVICE_VERSION_MINOR 1 #define LIBAVDEVICE_VERSION_MICRO 101 #define LIBAVDEVICE_VERSION_INT AV_VERSION_INT(LIBAVDEVICE_VERSION_MAJOR, \ -- 2.28.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 v5 10/13] avdevice/dshow: add media type info to get_device_list
The list returned by get_device_list now contains info about what media type(s), if any, can be provided by each device. Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index f25537db5c..fa3a06c077 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -377,6 +377,11 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, if (!device->device_name || !device->device_description) goto fail1; +device->nb_media_types = nb_media_types; +device->media_types = media_types; +nb_media_types = 0; +media_types = NULL; + // store to device_list output if (av_reallocp_array(&(*device_list)->devices, (*device_list)->nb_devices + 1, @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, ICreateDevEnum *devenum, av_freep(&device->device_name); if (device->device_name) av_freep(&device->device_description); +if (device->media_types) +av_freep(&device->media_types); av_free(device); } if (olestr && co_malloc) -- 2.28.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 v5 11/13] fftools: provide media type info for devices
fftools now print info about what media type(s), if any, are provided by sink and source avdevices. Signed-off-by: Diederick Niehorster --- fftools/cmdutils.c | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c index 3c8e5a82cd..7d7dcce2f9 100644 --- a/fftools/cmdutils.c +++ b/fftools/cmdutils.c @@ -2244,9 +2244,29 @@ double get_rotation(int32_t *displaymatrix) } #if CONFIG_AVDEVICE +static void print_device_list(const AVDeviceInfoList *device_list) +{ +// print devices +for (int i = 0; i < device_list->nb_devices; i++) { +const AVDeviceInfo *device = device_list->devices[i]; +printf("%s %s [%s] (", device_list->default_device == i ? "*" : " ", +device->device_name, device->device_description); +if (device->nb_media_types > 0 && device->media_types) { +for (int j = 0; j < device->nb_media_types; ++j) { +const char* media_type = av_get_media_type_string(device->media_types[j]); +if (j > 0) +printf(", "); +printf("%s", media_type ? media_type : "unknown"); +} +} else { +printf("none"); +} +printf(")\n"); +} +} static int print_device_sources(const AVInputFormat *fmt, AVDictionary *opts) { -int ret, i; +int ret; AVDeviceInfoList *device_list = NULL; if (!fmt || !fmt->priv_class || !AV_IS_INPUT_DEVICE(fmt->priv_class->category)) @@ -2258,10 +2278,7 @@ static int print_device_sources(const AVInputFormat *fmt, AVDictionary *opts) goto fail; } -for (i = 0; i < device_list->nb_devices; i++) { -printf("%c %s [%s]\n", device_list->default_device == i ? '*' : ' ', - device_list->devices[i]->device_name, device_list->devices[i]->device_description); -} +print_device_list(device_list); fail: avdevice_free_list_devices(&device_list); @@ -2270,7 +2287,7 @@ static int print_device_sources(const AVInputFormat *fmt, AVDictionary *opts) static int print_device_sinks(const AVOutputFormat *fmt, AVDictionary *opts) { -int ret, i; +int ret; AVDeviceInfoList *device_list = NULL; if (!fmt || !fmt->priv_class || !AV_IS_OUTPUT_DEVICE(fmt->priv_class->category)) @@ -2282,10 +2299,7 @@ static int print_device_sinks(const AVOutputFormat *fmt, AVDictionary *opts) goto fail; } -for (i = 0; i < device_list->nb_devices; i++) { -printf("%c %s [%s]\n", device_list->default_device == i ? '*' : ' ', - device_list->devices[i]->device_name, device_list->devices[i]->device_description); -} +print_device_list(device_list); fail: avdevice_free_list_devices(&device_list); -- 2.28.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 v5 12/13] avdevice/dshow: discover source color range/space/etc
Enabled discovering a DirectShow device's color range, space, primaries, transfer characteristics and chroma location, if the device exposes that information. Sets them in the stream's codecpars. Co-authored-by: Valerii Zapodovnikov Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 255 +++- 1 file changed, 254 insertions(+), 1 deletion(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index fa3a06c077..4ad6ae102c 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -29,6 +29,31 @@ #include "libavcodec/raw.h" #include "objidl.h" #include "shlwapi.h" +// NB: technically, we should include dxva.h and use +// DXVA_ExtendedFormat, but that type is not defined in +// the MinGW headers. The DXVA2_ExtendedFormat and the +// contents of its fields is identical to +// DXVA_ExtendedFormat (see https://docs.microsoft.com/en-us/windows/win32/medfound/extended-color-information#color-space-in-media-types) +// and is provided by MinGW as well, so we use that +// instead. NB also that per the Microsoft docs, the +// lowest 8 bits of the structure, i.e. the SampleFormat +// field, contain AMCONTROL_xxx flags instead of sample +// format information, and should thus not be used. +// NB further that various values in the structure's +// fields (e.g. BT.2020 color space) are not provided +// for either of the DXVA structs, but are provided in +// the flags of the corresponding fields of Media Foundation. +// These may be provided by DirectShow devices (e.g. LAVFilters +// does so). So we use those values here too (the equivalence is +// indicated by Microsoft example code: https://docs.microsoft.com/en-us/windows/win32/api/dxva2api/ns-dxva2api-dxva2_videodesc) +typedef DWORD D3DFORMAT;// dxva2api.h header needs these types defined before include apparently in WinSDK (not MinGW). +typedef DWORD D3DPOOL; +#include "dxva2api.h" + +#ifndef AMCONTROL_COLORINFO_PRESENT +// not defined in some versions of MinGW's dvdmedia.h +# define AMCONTROL_COLORINFO_PRESENT 0x0080 // if set, indicates DXVA color info is present in the upper (24) bits of the dwControlFlags +#endif static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount) @@ -54,6 +79,192 @@ static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount) return avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), biCompression); // all others } +static enum AVColorRange dshow_color_range(DXVA2_ExtendedFormat *fmt_info) +{ +switch (fmt_info->NominalRange) +{ +case DXVA2_NominalRange_Unknown: +return AVCOL_RANGE_UNSPECIFIED; +case DXVA2_NominalRange_Normal: // equal to DXVA2_NominalRange_0_255 +return AVCOL_RANGE_JPEG; +case DXVA2_NominalRange_Wide: // equal to DXVA2_NominalRange_16_235 +return AVCOL_RANGE_MPEG; +case DXVA2_NominalRange_48_208: +// not an ffmpeg color range +return AVCOL_RANGE_UNSPECIFIED; + +// values from MediaFoundation SDK (mfobjects.h) +case 4: // MFNominalRange_64_127 +// not an ffmpeg color range +return AVCOL_RANGE_UNSPECIFIED; + +default: +return DXVA2_NominalRange_Unknown; +} +} + +static enum AVColorSpace dshow_color_space(DXVA2_ExtendedFormat *fmt_info) +{ +enum AVColorSpace ret = AVCOL_SPC_UNSPECIFIED; + +switch (fmt_info->VideoTransferMatrix) +{ +case DXVA2_VideoTransferMatrix_BT709: +ret = AVCOL_SPC_BT709; +break; +case DXVA2_VideoTransferMatrix_BT601: +ret = AVCOL_SPC_BT470BG; +break; +case DXVA2_VideoTransferMatrix_SMPTE240M: +ret = AVCOL_SPC_SMPTE240M; +break; + +// values from MediaFoundation SDK (mfobjects.h) +case 4: // MFVideoTransferMatrix_BT2020_10 +case 5: // MFVideoTransferMatrix_BT2020_12 +if (fmt_info->VideoTransferFunction==12)// MFVideoTransFunc_2020_const +ret = AVCOL_SPC_BT2020_CL; +else +ret = AVCOL_SPC_BT2020_NCL; +break; +} + +if (ret == AVCOL_SPC_UNSPECIFIED) +{ +// if color space not known from transfer matrix, +// fall back to using primaries to guess color space +switch (fmt_info->VideoPrimaries) +{ +case DXVA2_VideoPrimaries_BT709: +ret = AVCOL_SPC_BT709; +break; +case DXVA2_VideoPrimaries_BT470_2_SysM: +ret = AVCOL_SPC_FCC; +break; +case DXVA2_VideoPrimaries_BT470_2_SysBG: +case DXVA2_VideoPrimaries_EBU3213: // this is PAL +ret = AVCOL_SPC_BT470BG; +break; +case DXVA2_VideoPrimaries_SMPTE170M: +case DXVA2_VideoPrimaries_SMPTE_C: +ret = AVCOL_SPC_SMPTE170M; +break; +case DXVA2_VideoPrimaries_SMPTE240M: +ret = AVCOL_SPC_SMPTE240M; +break; +} +} + +return ret; +} + +static enum AVColorPrimaries dshow_color_prim
[FFmpeg-devel] [PATCH v5 13/13] avdevice/dshow: select format with extended color info
Some DirectShow devices (Logitech C920 webcam) expose each DirectShow format they support twice, once without and once with extended color information. During format selection, both match, this patch ensures that the format with extended color information is selected if it is available, else it falls back to a matching format without such information. This also necessitated a new code path taken for default formats of a device (when user didn't request any specific video size, etc), because the default format may be one without extended color information when a twin with extended color information is also available. Getting the extended color information when available is important as it allows setting the color space, range, primaries, transfer characteristics and chroma location of the stream provided by dshow, enabling users to get more correct color automatically out of their device. Closes: #9271 Signed-off-by: Diederick Niehorster --- libavdevice/dshow.c | 469 +++- 1 file changed, 338 insertions(+), 131 deletions(-) diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c index 4ad6ae102c..6f2a3ac1ba 100644 --- a/libavdevice/dshow.c +++ b/libavdevice/dshow.c @@ -23,6 +23,7 @@ #include "libavutil/parseutils.h" #include "libavutil/pixdesc.h" #include "libavutil/opt.h" +#include "libavutil/mem.h" #include "libavformat/internal.h" #include "libavformat/riff.h" #include "avdevice.h" @@ -690,9 +691,111 @@ error: return ret; } +static int dshow_should_set_format(AVFormatContext *avctx, enum dshowDeviceType devtype) +{ +struct dshow_ctx *ctx = avctx->priv_data; + +return (devtype == VideoDevice && (ctx->framerate || + (ctx->requested_width && ctx->requested_height) || + ctx->pixel_format != AV_PIX_FMT_NONE || + ctx->video_codec_id != AV_CODEC_ID_RAWVIDEO)) +|| (devtype == AudioDevice && (ctx->channels || ctx->sample_size || ctx->sample_rate)); +} + + +struct dshow_format_info { +enum dshowDeviceType devtype; +// video +int64_t framerate; +enum AVPixelFormat pix_fmt; +enum AVCodecID codec_id; +enum AVColorRange col_range; +enum AVColorSpace col_space; +enum AVColorPrimaries col_prim; +enum AVColorTransferCharacteristic col_trc; +enum AVChromaLocation chroma_loc; +int width; +int height; +// audio +int sample_rate; +int sample_size; +int channels; +}; + +// user must av_free the returned pointer +static struct dshow_format_info *dshow_get_format_info(AM_MEDIA_TYPE *type) +{ +struct dshow_format_info *fmt_info = NULL; +BITMAPINFOHEADER *bih; +DXVA2_ExtendedFormat *extended_format_info = NULL; +WAVEFORMATEX *fx; +enum dshowDeviceType devtype; +int64_t framerate; + +if (!type) +return NULL; + +if (IsEqualGUID(&type->formattype, &FORMAT_VideoInfo)) { +VIDEOINFOHEADER *v = (void *) type->pbFormat; +framerate = v->AvgTimePerFrame; +bih = &v->bmiHeader; +devtype = VideoDevice; +} else if (IsEqualGUID(&type->formattype, &FORMAT_VideoInfo2)) { +VIDEOINFOHEADER2 *v = (void *) type->pbFormat; +devtype = VideoDevice; +framerate = v->AvgTimePerFrame; +bih = &v->bmiHeader; +if (v->dwControlFlags & AMCONTROL_COLORINFO_PRESENT) +extended_format_info = (DXVA2_ExtendedFormat *) &v->dwControlFlags; +} else if (IsEqualGUID(&type->formattype, &FORMAT_WaveFormatEx)) { +fx = (void *) type->pbFormat; +devtype = AudioDevice; +} else { +return NULL; +} + +fmt_info = av_mallocz(sizeof(struct dshow_format_info)); +if (!fmt_info) +return NULL; +// initialize fields where unset is not zero +fmt_info->pix_fmt = AV_PIX_FMT_NONE; +fmt_info->col_space = AVCOL_SPC_UNSPECIFIED; +fmt_info->col_prim = AVCOL_PRI_UNSPECIFIED; +fmt_info->col_trc = AVCOL_TRC_UNSPECIFIED; +// now get info about format +fmt_info->devtype = devtype; +if (devtype == VideoDevice) { +fmt_info->width = bih->biWidth; +fmt_info->height = bih->biHeight; +fmt_info->framerate = framerate; +fmt_info->pix_fmt = dshow_pixfmt(bih->biCompression, bih->biBitCount); +if (fmt_info->pix_fmt == AV_PIX_FMT_NONE) { +const AVCodecTag *const tags[] = { avformat_get_riff_video_tags(), NULL }; +fmt_info->codec_id = av_codec_get_id(tags, bih->biCompression); +} +else +fmt_info->codec_id = AV_CODEC_ID_RAWVIDEO; + +if (extended_format_info) { +fmt_info->col_range = dshow_color_range(extended_format_info); +fmt_info->col_space = dshow_color_space(extended_format_info); +fmt_info->col_prim = dshow_color_primaries(extended_format_info); +fmt_info->col_trc = dshow_color_trc(extended
Re: [FFmpeg-devel] [PATCH] avutil: add objc.h to SKIPHEADERS
On Sun, Dec 19, 2021 at 9:41 AM Aman Karmani wrote: > From: Aman Karmani > > Signed-off-by: Aman Karmani > --- > libavutil/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavutil/Makefile b/libavutil/Makefile > index 529046dbc8..d17876df1a 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -195,6 +195,7 @@ OBJS += $(COMPAT_OBJS:%=../compat/%) > # Windows resource file > SLIBOBJS-$(HAVE_GNU_WINDRES)+= avutilres.o > > +SKIPHEADERS+= objc.h Applied to fix `make checkheaders` > SKIPHEADERS-$(HAVE_CUDA_H) += hwcontext_cuda.h > SKIPHEADERS-$(CONFIG_CUDA) += hwcontext_cuda_internal.h \ >cuda_check.h > -- > 2.33.0 > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability
Enables creation of FLAC files with up to 32 bits-per-sample, up from the previous limit of 24 bit. This is a feature requested for RAWcooked, the archiving community has a need for storing files with 32-bit integer audio samples. See https://github.com/MediaArea/RAWcooked/issues/356 Restrictions to the encoder are added so created files are compatible with existing decoders. Stereo decorrelation is disabled on 32 bit-per-sample, because a side channel would need 33 bit-per-sample, causing problems in existing 32 bit datapaths. Also only LPC encoding is enabled, because decoders capable of processing 24-bit files already use 64-bit processing for LPC, but not for fixed subframes. Furthermore, predictions and residuals are checked for causing integer overflow, reverting to a verbatim (store) subframe in case no LPC coeffs can be found that do not cause overflow. ffmpeg's FLAC decoder has been forward-compatible with this change since commit c720b9ce98 (May 2015). libFLAC is forward-compatible since release 1.2.1 (September 2007), the flac command line tool however blocks 32-bit files out of caution, it having been untested until now. --- libavcodec/flacdsp.c | 47 ++ libavcodec/flacdsp.h | 3 ++ libavcodec/flacenc.c | 95 +--- 3 files changed, 130 insertions(+), 15 deletions(-) diff --git a/libavcodec/flacdsp.c b/libavcodec/flacdsp.c index bc9a5dbed9..422f6578ba 100644 --- a/libavcodec/flacdsp.c +++ b/libavcodec/flacdsp.c @@ -43,6 +43,53 @@ #define PLANAR 1 #include "flacdsp_template.c" +#define ZIGZAG_32BIT_MAX 0x3FFF +#define ZIGZAG_32BIT_MIN -0x3FFF + +int ff_flacdsp_lpc_encode_c_32_overflow_detect(int32_t *res, const int32_t *smp, int len, + int order, int32_t *coefs, int shift) +{ +/* This function checks for every prediction and every residual + * whether they cause integer overflow in existing decoders. In + * case the prediction exceeds int32_t limits, prediction + * coefficients are lowered accordingly. If the residual exceeds + * ZIGZAG_32BIT_MAX and _MIN or coefficients have been lowered + * twice but the prediction still overflows, give up */ +int lpc_reduction_tries = 0; +int64_t pmax; +for (int i = 0; i < order; i++) +res[i] = smp[i]; +do { +pmax = 0; +for (int i = order; i < len; i++) { +int64_t p = 0, tmp; +for (int j = 0; j < order; j++) +p += (int64_t)coefs[j]*smp[(i-1)-j]; +p >>= shift; +tmp = smp[i] - p; +if (p > INT32_MAX && p > pmax) +pmax = p; +else if (p < INT32_MIN && (p * -1) > pmax) +pmax = p * -1; +if (tmp > ZIGZAG_32BIT_MAX || tmp < ZIGZAG_32BIT_MIN) +return 0; +res[i] = tmp; +} + +if (pmax > 0) { +double scale; +if(lpc_reduction_tries >= 2) +return 0; +lpc_reduction_tries++; +scale = (double)INT32_MAX/pmax; +for (int i = 0; i < order; i++) +coefs[i] *= scale; +} +} while (pmax > 0); +return 1; +} + + static void flac_lpc_16_c(int32_t *decoded, const int coeffs[32], int pred_order, int qlevel, int len) { diff --git a/libavcodec/flacdsp.h b/libavcodec/flacdsp.h index 7bb0dd0e9a..5978a4722a 100644 --- a/libavcodec/flacdsp.h +++ b/libavcodec/flacdsp.h @@ -40,4 +40,7 @@ void ff_flacdsp_init(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, i void ff_flacdsp_init_arm(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps); void ff_flacdsp_init_x86(FLACDSPContext *c, enum AVSampleFormat fmt, int channels, int bps); +int ff_flacdsp_lpc_encode_c_32_overflow_detect(int32_t *res, const int32_t *smp, int len, + int order, int32_t *coefs, int shift); + #endif /* AVCODEC_FLACDSP_H */ diff --git a/libavcodec/flacenc.c b/libavcodec/flacenc.c index 595928927d..64d4b8a71d 100644 --- a/libavcodec/flacenc.c +++ b/libavcodec/flacenc.c @@ -254,10 +254,30 @@ static av_cold int flac_encode_init(AVCodecContext *avctx) s->bps_code= 4; break; case AV_SAMPLE_FMT_S32: -if (avctx->bits_per_raw_sample != 24) -av_log(avctx, AV_LOG_WARNING, "encoding as 24 bits-per-sample\n"); -avctx->bits_per_raw_sample = 24; -s->bps_code= 6; +if (avctx->bits_per_raw_sample > 0 && avctx->bits_per_raw_sample <= 24){ +if(avctx->bits_per_raw_sample < 24) +av_log(avctx, AV_LOG_WARNING, "encoding as 24 bits-per-sample\n"); +avctx->bits_per_raw_sample = 24; +s->bps_code= 6; +} else { +av_log(avctx, AV_LOG_WARNING, "non-streamable bits-per-sample\n"); +s->bps_code = 0
Re: [FFmpeg-devel] [PATCH] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability
19 Dec 2021, 21:53 by mva...@gmail.com: > Enables creation of FLAC files with up to 32 bits-per-sample, up from the > previous limit of 24 bit. This is a feature requested for RAWcooked, the > archiving community has a need for storing files with 32-bit integer audio > samples. See https://github.com/MediaArea/RAWcooked/issues/356 > > Restrictions to the encoder are added so created files are compatible with > existing decoders. Stereo decorrelation is disabled on 32 bit-per-sample, > because a side channel would need 33 bit-per-sample, causing problems in > existing 32 bit datapaths. Also only LPC encoding is enabled, because > decoders capable of processing 24-bit files already use 64-bit processing > for LPC, but not for fixed subframes. > > Furthermore, predictions and residuals are checked for causing integer > overflow, reverting to a verbatim (store) subframe in case no LPC coeffs > can be found that do not cause overflow. > > ffmpeg's FLAC decoder has been forward-compatible with this change since > commit c720b9ce98 (May 2015). libFLAC is forward-compatible since release > 1.2.1 (September 2007), the flac command line tool however blocks 32-bit > files out of caution, it having been untested until now. > --- > libavcodec/flacdsp.c | 47 ++ > libavcodec/flacdsp.h | 3 ++ > libavcodec/flacenc.c | 95 +--- > 3 files changed, 130 insertions(+), 15 deletions(-) > > diff --git a/libavcodec/flacdsp.c b/libavcodec/flacdsp.c > index bc9a5dbed9..422f6578ba 100644 > --- a/libavcodec/flacdsp.c > +++ b/libavcodec/flacdsp.c > @@ -43,6 +43,53 @@ > #define PLANAR 1 > #include "flacdsp_template.c" > > +#define ZIGZAG_32BIT_MAX 0x3FFF > +#define ZIGZAG_32BIT_MIN -0x3FFF > + > +int ff_flacdsp_lpc_encode_c_32_overflow_detect(int32_t *res, const int32_t > *smp, int len, > + int order, int32_t *coefs, > int shift) > +{ > +/* This function checks for every prediction and every residual > + * whether they cause integer overflow in existing decoders. In > + * case the prediction exceeds int32_t limits, prediction > + * coefficients are lowered accordingly. If the residual exceeds > + * ZIGZAG_32BIT_MAX and _MIN or coefficients have been lowered > + * twice but the prediction still overflows, give up */ > What happens if there's an overflow and the prediction coefficients are lowered? Is there a loss of bits? What about if it gives up? I'm really concerned about arbitrary data not being lossless in a codec that's meant to be always lossless. If that can happen, I think 32-bit encoding should be disabled unless -strict -1 is used. ___ 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] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability
Op zo 19 dec. 2021 om 22:11 schreef Lynne : > What happens if there's an overflow and the prediction coefficients > are lowered? Is there a loss of bits? What about if it gives up? The result remains lossless. If the prediction coefficients are lowered, the residual of the prediction increases. This means the file gets a little bigger, but the data is still all there. If it gives up on searching for suitable LPC coefficients, it uses a verbatim subframe instead of an LPC subframe. That means all data is stored as unencoded PCM, with no form of prediction whatsoever. This results in (usually a large) increase in storage space needed, but this is still lossless. ___ 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] libavcodec/flacenc: add backward-compatible 32 bit-per-sample capability
19 Dec 2021, 22:41 by mva...@gmail.com: > Op zo 19 dec. 2021 om 22:11 schreef Lynne : > >> What happens if there's an overflow and the prediction coefficients >> are lowered? Is there a loss of bits? What about if it gives up? >> > > The result remains lossless. If the prediction coefficients are > lowered, the residual of the prediction increases. This means the file > gets a little bigger, but the data is still all there. > > If it gives up on searching for suitable LPC coefficients, it uses a > verbatim subframe instead of an LPC subframe. That means all data is > stored as unencoded PCM, with no form of prediction whatsoever. This > results in (usually a large) increase in storage space needed, but > this is still lossless. > That's reasonable. Some code style issues: > + for (i = 0; i < s->frame.blocksize * s->channels; i++) { > + AV_WL32(tmp + 4*i, samples0[i]); > + } No brackets here. > + if(!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, > n, opt_order, > + lpc_try, > shift[opt_order-1])) > + continue; Put a space after the if. > + if(!ff_flacdsp_lpc_encode_c_32_overflow_detect(res, smp, > n, i+1, > + coefs[i], > shift[i])) > + continue; And here, and in 2 other places. > + if (pmax > 0) { > + double scale; > + if(lpc_reduction_tries >= 2) Space after if. > + return 0; > + lpc_reduction_tries++; > + scale = (double)INT32_MAX/pmax; > + for (int i = 0; i < order; i++) > + coefs[i] *= scale; > + } Could you replace this with a fixed-point implementation as well? Even if it's outside of the audio path, we already have one issue in the flac code where using a double in assembly is causing a test to fail. I still have to rewrite that from inline assembly to proper external asm. ___ 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/ass: Faster ff_ass_add_rect()
Signed-off-by: Michael Niedermayer --- libavcodec/ass.c | 33 +++-- libavcodec/ass.h | 7 +++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/libavcodec/ass.c b/libavcodec/ass.c index 725e4d42ba1..06714678722 100644 --- a/libavcodec/ass.c +++ b/libavcodec/ass.c @@ -114,17 +114,31 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, speaker ? speaker : "", text); } -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, -const char *speaker) +const char *speaker, unsigned *nb_rect_allocated) { -AVSubtitleRect **rects, *rect; +AVSubtitleRect **rects = sub->rects, *rect; char *ass_str; +uint64_t new_nb = 0; -rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); -if (!rects) +if (sub->num_rects >= UINT_MAX) return AVERROR(ENOMEM); -sub->rects = rects; + +if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { +new_nb = FFMIN(sub->num_rects + sub->num_rects/16LL + 1, UINT_MAX); +} else if (!nb_rect_allocated) +new_nb = sub->num_rects + 1LL; + +if (new_nb) { +rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects)); +if (!rects) +return AVERROR(ENOMEM); +if (nb_rect_allocated) +*nb_rect_allocated = new_nb; +sub->rects = rects; +} + rect = av_mallocz(sizeof(*rect)); if (!rect) return AVERROR(ENOMEM); @@ -137,6 +151,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, return 0; } +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, +int readorder, int layer, const char *style, +const char *speaker) +{ +return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, NULL); +} + void ff_ass_decoder_flush(AVCodecContext *avctx) { FFASSDecoderContext *s = avctx->priv_data; diff --git a/libavcodec/ass.h b/libavcodec/ass.h index 2c260e4e785..4dffe923d9f 100644 --- a/libavcodec/ass.h +++ b/libavcodec/ass.h @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, const char *speaker); +/** + * Add an ASS dialog to a subtitle. + */ +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, + int readorder, int layer, const char *style, + const char *speaker, unsigned *nb_rect_allocated); + /** * Helper to flush a text subtitles decoder making use of the * FFASSDecoderContext. -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avcodec/ccaption_dec: Use ff_ass_add_rect2()
Fixes: Timeout Fixes: 42258/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CCAPTION_fuzzer-5540144118104064 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/ccaption_dec.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c index 27c61527f6e..15be18eb164 100644 --- a/libavcodec/ccaption_dec.c +++ b/libavcodec/ccaption_dec.c @@ -850,6 +850,7 @@ static int decode(AVCodecContext *avctx, void *data, int *got_sub, AVPacket *avp int len = avpkt->size; int ret = 0; int i; +unsigned nb_rect_allocated = 0; for (i = 0; i < len; i += 3) { uint8_t hi, cc_type = bptr[i] & 1; @@ -886,7 +887,7 @@ static int decode(AVCodecContext *avctx, void *data, int *got_sub, AVPacket *avp AV_TIME_BASE_Q, ms_tb); else sub->end_display_time = -1; -ret = ff_ass_add_rect(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL); +ret = ff_ass_add_rect2(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL, &nb_rect_allocated); if (ret < 0) return ret; ctx->last_real_time = sub->pts; @@ -896,7 +897,7 @@ static int decode(AVCodecContext *avctx, void *data, int *got_sub, AVPacket *avp if (!bptr && !ctx->real_time && ctx->buffer[!ctx->buffer_index].str[0]) { bidx = !ctx->buffer_index; -ret = ff_ass_add_rect(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL); +ret = ff_ass_add_rect2(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL, &nb_rect_allocated); if (ret < 0) return ret; sub->pts = ctx->buffer_time[1]; @@ -914,7 +915,7 @@ static int decode(AVCodecContext *avctx, void *data, int *got_sub, AVPacket *avp capture_screen(ctx); ctx->buffer_changed = 0; -ret = ff_ass_add_rect(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL); +ret = ff_ass_add_rect2(sub, ctx->buffer[bidx].str, ctx->readorder++, 0, NULL, NULL, &nb_rect_allocated); if (ret < 0) return ret; sub->end_display_time = -1; -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] avcodec/vqavideo: reset accounting on error
Fixes: Timeout (same growing chunk is decoded to failure repeatedly) Fixes: 42582/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6531195591065600 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/vqavideo.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c index 7c1d42bcacc..1d97855e606 100644 --- a/libavcodec/vqavideo.c +++ b/libavcodec/vqavideo.c @@ -608,13 +608,14 @@ static int vqa_decode_frame_pal8(VqaContext *s, AVFrame *frame) if (s->partial_countdown <= 0) { bytestream2_init(&s->gb, s->next_codebook_buffer, s->next_codebook_buffer_index); /* decompress codebook */ -if ((res = decode_format80(s, s->next_codebook_buffer_index, - s->codebook, s->codebook_size, 0)) < 0) -return res; +res = decode_format80(s, s->next_codebook_buffer_index, + s->codebook, s->codebook_size, 0); /* reset accounting */ s->next_codebook_buffer_index = 0; s->partial_countdown = s->partial_count; +if (res < 0) +return res; } } -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/ass: Faster ff_ass_add_rect()
On 12/19/2021 8:56 PM, Michael Niedermayer wrote: Signed-off-by: Michael Niedermayer --- libavcodec/ass.c | 33 +++-- libavcodec/ass.h | 7 +++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/libavcodec/ass.c b/libavcodec/ass.c index 725e4d42ba1..06714678722 100644 --- a/libavcodec/ass.c +++ b/libavcodec/ass.c @@ -114,17 +114,31 @@ char *ff_ass_get_dialog(int readorder, int layer, const char *style, speaker ? speaker : "", text); } -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, -const char *speaker) +const char *speaker, unsigned *nb_rect_allocated) { -AVSubtitleRect **rects, *rect; +AVSubtitleRect **rects = sub->rects, *rect; char *ass_str; +uint64_t new_nb = 0; -rects = av_realloc_array(sub->rects, sub->num_rects+1, sizeof(*sub->rects)); -if (!rects) +if (sub->num_rects >= UINT_MAX) return AVERROR(ENOMEM); -sub->rects = rects; + +if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { +new_nb = FFMIN(sub->num_rects + sub->num_rects/16LL + 1, UINT_MAX); Isn't this what av_fast_realloc() is for? +} else if (!nb_rect_allocated) +new_nb = sub->num_rects + 1LL; + +if (new_nb) { +rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects)); +if (!rects) +return AVERROR(ENOMEM); +if (nb_rect_allocated) +*nb_rect_allocated = new_nb; +sub->rects = rects; +} + rect = av_mallocz(sizeof(*rect)); if (!rect) return AVERROR(ENOMEM); @@ -137,6 +151,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, return 0; } +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, +int readorder, int layer, const char *style, +const char *speaker) +{ +return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, NULL); +} + void ff_ass_decoder_flush(AVCodecContext *avctx) { FFASSDecoderContext *s = avctx->priv_data; diff --git a/libavcodec/ass.h b/libavcodec/ass.h index 2c260e4e785..4dffe923d9f 100644 --- a/libavcodec/ass.h +++ b/libavcodec/ass.h @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, int readorder, int layer, const char *style, const char *speaker); +/** + * Add an ASS dialog to a subtitle. + */ +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, + int readorder, int layer, const char *style, + const char *speaker, unsigned *nb_rect_allocated); + /** * Helper to flush a text subtitles decoder making use of the * FFASSDecoderContext. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/ass: Faster ff_ass_add_rect()
Michael Niedermayer: > Signed-off-by: Michael Niedermayer > --- > libavcodec/ass.c | 33 +++-- > libavcodec/ass.h | 7 +++ > 2 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/libavcodec/ass.c b/libavcodec/ass.c > index 725e4d42ba1..06714678722 100644 > --- a/libavcodec/ass.c > +++ b/libavcodec/ass.c > @@ -114,17 +114,31 @@ char *ff_ass_get_dialog(int readorder, int layer, const > char *style, > speaker ? speaker : "", text); > } > > -int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, > int readorder, int layer, const char *style, > -const char *speaker) > +const char *speaker, unsigned *nb_rect_allocated) > { > -AVSubtitleRect **rects, *rect; > +AVSubtitleRect **rects = sub->rects, *rect; > char *ass_str; > +uint64_t new_nb = 0; > > -rects = av_realloc_array(sub->rects, sub->num_rects+1, > sizeof(*sub->rects)); > -if (!rects) > +if (sub->num_rects >= UINT_MAX) > return AVERROR(ENOMEM); > -sub->rects = rects; > + > +if (nb_rect_allocated && *nb_rect_allocated <= sub->num_rects) { > +new_nb = FFMIN(sub->num_rects + sub->num_rects/16LL + 1, UINT_MAX); This presumes that unsigned is not 64bits itself; I have no problem with this, so LGTM from me. Others may disagree. > +} else if (!nb_rect_allocated) > +new_nb = sub->num_rects + 1LL; +1 is enough (it has been checked that sub->num_rects is < UINT_MAX). > + > +if (new_nb) { > +rects = av_realloc_array(rects, new_nb, sizeof(*sub->rects)); > +if (!rects) > +return AVERROR(ENOMEM); > +if (nb_rect_allocated) > +*nb_rect_allocated = new_nb; > +sub->rects = rects; > +} > + > rect = av_mallocz(sizeof(*rect)); > if (!rect) > return AVERROR(ENOMEM); > @@ -137,6 +151,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > return 0; > } > > +int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > +int readorder, int layer, const char *style, > +const char *speaker) > +{ > +return ff_ass_add_rect2(sub, dialog, readorder, layer, style, speaker, > NULL); > +} > + > void ff_ass_decoder_flush(AVCodecContext *avctx) > { > FFASSDecoderContext *s = avctx->priv_data; > diff --git a/libavcodec/ass.h b/libavcodec/ass.h > index 2c260e4e785..4dffe923d9f 100644 > --- a/libavcodec/ass.h > +++ b/libavcodec/ass.h > @@ -118,6 +118,13 @@ int ff_ass_add_rect(AVSubtitle *sub, const char *dialog, > int readorder, int layer, const char *style, > const char *speaker); > > +/** > + * Add an ASS dialog to a subtitle. > + */ > +int ff_ass_add_rect2(AVSubtitle *sub, const char *dialog, > + int readorder, int layer, const char *style, > + const char *speaker, unsigned *nb_rect_allocated); > + > /** > * Helper to flush a text subtitles decoder making use of the > * FFASSDecoderContext. > ___ 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 v5 03/13] avdevice/dshow: add use_video_device_timestamps to docs
Diederick Niehorster: > Signed-off-by: Diederick Niehorster > --- > doc/indevs.texi | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/doc/indevs.texi b/doc/indevs.texi > index 5be647f70a..9d8020311a 100644 > --- a/doc/indevs.texi > +++ b/doc/indevs.texi > @@ -625,6 +625,12 @@ Save the currently used video capture filter device and > its > parameters (if the filter supports it) to a file. > If a file with the same name exists it will be overwritten. > > +@item use_video_device_timestamps > +If set to @option{false}, the timestamp for video frames will be > +derived from the wallclock instead of the timestamp provided by > +the capture device. This allows working around devices that > +provide unreliable timestamps. > + > @end table > > @subsection Examples > Documentation for new options should be added in the same patch that adds the option. - 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 v5 08/13] avdevice/dshow: list_devices: show media type(s) per device
Diederick Niehorster: > the list_devices option of dshow didn't indicate whether a specific > device provides audio or video output. This patch iterates through all > media formats of all pins exposed by the device to see what types it > provides for capture, and prints this to the console for each device. > Importantly, this now allows to find devices that provide both audio and > video, and devices that provide neither. > > Signed-off-by: Diederick Niehorster > --- > libavdevice/dshow.c | 103 +--- > 1 file changed, 98 insertions(+), 5 deletions(-) > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > index 8c257ca8e7..f25537db5c 100644 > --- a/libavdevice/dshow.c > +++ b/libavdevice/dshow.c > @@ -197,6 +197,79 @@ fail: > return; > } > > +static void dshow_get_device_media_types(AVFormatContext *avctx, enum > dshowDeviceType devtype, > + enum dshowSourceFilterType > sourcetype, IBaseFilter *device_filter, > + enum AVMediaType **media_types, int > *nb_media_types) > +{ > +struct dshow_ctx *ctx = avctx->priv_data; > +IEnumPins *pins = 0; > +IPin *pin; > +int has_audio = 0, has_video = 0; > + > +if (IBaseFilter_EnumPins(device_filter, &pins) != S_OK) > +return; > + > +while (IEnumPins_Next(pins, 1, &pin, NULL) == S_OK) { > +IKsPropertySet *p = NULL; > +PIN_INFO info = { 0 }; > +GUID category; > +DWORD r2; > +IEnumMediaTypes *types = NULL; > +AM_MEDIA_TYPE *type; > + > +if (IPin_QueryPinInfo(pin, &info) != S_OK) > +goto next; > +IBaseFilter_Release(info.pFilter); > + > +if (info.dir != PINDIR_OUTPUT) > +goto next; > +if (IPin_QueryInterface(pin, &IID_IKsPropertySet, (void **) &p) != > S_OK) > +goto next; > +if (IKsPropertySet_Get(p, &ROPSETID_Pin, AMPROPERTY_PIN_CATEGORY, > + NULL, 0, &category, sizeof(GUID), &r2) != > S_OK) > +goto next; > +if (!IsEqualGUID(&category, &PIN_CATEGORY_CAPTURE)) > +goto next; > + > +if (IPin_EnumMediaTypes(pin, &types) != S_OK) > +goto next; > + > +// enumerate media types exposed by pin > +// NB: don't know if a pin can expose both audio and video, check 'm > all to be safe > +IEnumMediaTypes_Reset(types); > +while (IEnumMediaTypes_Next(types, 1, &type, NULL) == S_OK) { > +if (IsEqualGUID(&type->majortype, &MEDIATYPE_Video)) { > +has_video = 1; > +} else if (IsEqualGUID(&type->majortype, &MEDIATYPE_Audio)) { > +has_audio = 1; > +} > +CoTaskMemFree(type); > +} > + > +next: > +if (types) > +IEnumMediaTypes_Release(types); > +if (p) > +IKsPropertySet_Release(p); > +if (pin) > +IPin_Release(pin); > +} > + > +IEnumPins_Release(pins); > + > +if (has_audio || has_video) { > +int nb_types = has_audio + has_video; > +*media_types = av_malloc_array(nb_types, sizeof(enum AVMediaType)); You are allocating a list of at most two entries; why not put an array of that many entries on the stack in dshow_cycle_devices() and write them there. This would get rid of the allocations, the frees and would fix the problem that you do not forward the error in case of allocation failure. (The required size of said array should of course be defined by a macro.) > +if (*media_types) { > +if (has_audio) > +(*media_types)[0] = AVMEDIA_TYPE_AUDIO; > +if (has_video) > +(*media_types)[0 + has_audio] = AVMEDIA_TYPE_VIDEO; > +*nb_media_types = nb_types; > +} > +} > +} > + > /** > * Cycle through available devices using the device enumerator devenum, > * retrieve the device with type specified by devtype and return the > @@ -242,6 +315,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > ICreateDevEnum *devenum, > LPOLESTR olestr = NULL; > LPMALLOC co_malloc = NULL; > AVDeviceInfo *device = NULL; > +enum AVMediaType *media_types = NULL; > +int nb_media_types = 0; > int i; > > r = CoGetMalloc(1, &co_malloc); > @@ -286,6 +361,12 @@ dshow_cycle_devices(AVFormatContext *avctx, > ICreateDevEnum *devenum, > // success, loop will end now > } > } else { > +// get media types exposed by pins of device > +if (IMoniker_BindToObject(m, 0, 0, &IID_IBaseFilter, (void* ) > &device_filter) == S_OK) { > +dshow_get_device_media_types(avctx, devtype, sourcetype, > device_filter, &media_types, &nb_media_types); > +IBaseFilter_Release(device_filter); > +device_filter = NULL; > +
Re: [FFmpeg-devel] [PATCH v5 09/13] avdevice: add info about media types(s) to AVDeviceInfo
Diederick Niehorster: > An avdevice, regardless of whether its category says its an audio or > video device, may provide access to devices providing different media > types, or even single devices providing multiple media types. Also, some > devices may provide no media types. dshow is an example encompassing all > of these cases. Users should be provided with this information, so > AVDeviceInfo is extended to provide it. > > Bump avdevice version > > Signed-off-by: Diederick Niehorster > --- > libavdevice/avdevice.c | 2 ++ > libavdevice/avdevice.h | 2 ++ > libavdevice/version.h | 2 +- > 3 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c > index 2ae26ab8e3..712ef1e80c 100644 > --- a/libavdevice/avdevice.c > +++ b/libavdevice/avdevice.c > @@ -157,6 +157,8 @@ void avdevice_free_list_devices(AVDeviceInfoList > **device_list) > if (dev) { > av_freep(&dev->device_name); > av_freep(&dev->device_description); > +if (dev->media_types) > +av_freep(&dev->media_types); av_freep() can handle the case of dev->media_types == NULL just fine, so the check can be removed (yes, this might be a tiny bit slower in case dev->media_types is NULL, but this is not hot code). > av_free(dev); > } > } > diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h > index 8370bbc7f2..6f24976dcc 100644 > --- a/libavdevice/avdevice.h > +++ b/libavdevice/avdevice.h > @@ -457,6 +457,8 @@ void avdevice_capabilities_free(AVDeviceCapabilitiesQuery > **caps, AVFormatContex > typedef struct AVDeviceInfo { > char *device_name; /**< device name, format depends on > device */ > char *device_description;/**< human friendly name */ > +enum AVMediaType *media_types; /**< array indicating what media > types(s), if any, a device can provide. If null, cannot provide any */ > +int nb_media_types; /**< length of media_types array, 0 > if device cannot provide any media types */ Personally, I'd prefer it if this were unsigned given that negative values don't make sense. But this is just a personal preference. > } AVDeviceInfo; > > /** > diff --git a/libavdevice/version.h b/libavdevice/version.h > index 914e156ec7..c549768e12 100644 > --- a/libavdevice/version.h > +++ b/libavdevice/version.h > @@ -28,7 +28,7 @@ > #include "libavutil/version.h" > > #define LIBAVDEVICE_VERSION_MAJOR 59 > -#define LIBAVDEVICE_VERSION_MINOR 0 > +#define LIBAVDEVICE_VERSION_MINOR 1 > #define LIBAVDEVICE_VERSION_MICRO 101 MICRO should be reset if MINOR is bumped. > > #define LIBAVDEVICE_VERSION_INT AV_VERSION_INT(LIBAVDEVICE_VERSION_MAJOR, \ > ___ 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 v5 10/13] avdevice/dshow: add media type info to get_device_list
Diederick Niehorster: > The list returned by get_device_list now contains info about what media > type(s), if any, can be provided by each device. > > Signed-off-by: Diederick Niehorster > --- > libavdevice/dshow.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > index f25537db5c..fa3a06c077 100644 > --- a/libavdevice/dshow.c > +++ b/libavdevice/dshow.c > @@ -377,6 +377,11 @@ dshow_cycle_devices(AVFormatContext *avctx, > ICreateDevEnum *devenum, > if (!device->device_name || !device->device_description) > goto fail1; > > +device->nb_media_types = nb_media_types; > +device->media_types = media_types; > +nb_media_types = 0; > +media_types = NULL; > + If this array is intended to be given to the caller, my "put in on the stack" suggestion from #8 is no longer feasible. > // store to device_list output > if (av_reallocp_array(&(*device_list)->devices, > (*device_list)->nb_devices + 1, > @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > ICreateDevEnum *devenum, > av_freep(&device->device_name); > if (device->device_name) > av_freep(&device->device_description); > +if (device->media_types) > +av_freep(&device->media_types); You are duplicating freeing code here: You have code to free media_types both before it was put into device and after; you can avoid the latter by only attaching media_types to device when nothing else can fail. Btw: All these checks before av_freep() here are unnecessary. > av_free(device); > } > if (olestr && co_malloc) > ___ 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 v5 11/13] fftools: provide media type info for devices
Diederick Niehorster: > fftools now print info about what media type(s), if any, are provided by > sink and source avdevices. > > Signed-off-by: Diederick Niehorster > --- > fftools/cmdutils.c | 34 -- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c > index 3c8e5a82cd..7d7dcce2f9 100644 > --- a/fftools/cmdutils.c > +++ b/fftools/cmdutils.c > @@ -2244,9 +2244,29 @@ double get_rotation(int32_t *displaymatrix) > } > > #if CONFIG_AVDEVICE > +static void print_device_list(const AVDeviceInfoList *device_list) > +{ > +// print devices > +for (int i = 0; i < device_list->nb_devices; i++) { > +const AVDeviceInfo *device = device_list->devices[i]; > +printf("%s %s [%s] (", device_list->default_device == i ? "*" : " ", The first string can be written as char (as it is now). > +device->device_name, device->device_description); > +if (device->nb_media_types > 0 && device->media_types) { You are checking for both the counter as well as the pointer. This is unnecessary if libavdevice does its job properly and it could mask situations in which it fails to do so. > +for (int j = 0; j < device->nb_media_types; ++j) { > +const char* media_type = > av_get_media_type_string(device->media_types[j]); > +if (j > 0) > +printf(", "); > +printf("%s", media_type ? media_type : "unknown"); > +} > +} else { > +printf("none"); > +} > +printf(")\n"); > +} > +} > static int print_device_sources(const AVInputFormat *fmt, AVDictionary *opts) > { > -int ret, i; > +int ret; > AVDeviceInfoList *device_list = NULL; > > if (!fmt || !fmt->priv_class || > !AV_IS_INPUT_DEVICE(fmt->priv_class->category)) > @@ -2258,10 +2278,7 @@ static int print_device_sources(const AVInputFormat > *fmt, AVDictionary *opts) > goto fail; > } > > -for (i = 0; i < device_list->nb_devices; i++) { > -printf("%c %s [%s]\n", device_list->default_device == i ? '*' : ' ', > - device_list->devices[i]->device_name, > device_list->devices[i]->device_description); > -} > +print_device_list(device_list); > >fail: > avdevice_free_list_devices(&device_list); > @@ -2270,7 +2287,7 @@ static int print_device_sources(const AVInputFormat > *fmt, AVDictionary *opts) > > static int print_device_sinks(const AVOutputFormat *fmt, AVDictionary *opts) > { > -int ret, i; > +int ret; > AVDeviceInfoList *device_list = NULL; > > if (!fmt || !fmt->priv_class || > !AV_IS_OUTPUT_DEVICE(fmt->priv_class->category)) > @@ -2282,10 +2299,7 @@ static int print_device_sinks(const AVOutputFormat > *fmt, AVDictionary *opts) > goto fail; > } > > -for (i = 0; i < device_list->nb_devices; i++) { > -printf("%c %s [%s]\n", device_list->default_device == i ? '*' : ' ', > - device_list->devices[i]->device_name, > device_list->devices[i]->device_description); > -} > +print_device_list(device_list); > >fail: > avdevice_free_list_devices(&device_list); > ___ 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 v5 12/13] avdevice/dshow: discover source color range/space/etc
Diederick Niehorster: > Enabled discovering a DirectShow device's color range, space, primaries, > transfer characteristics and chroma location, if the device exposes that > information. Sets them in the stream's codecpars. > > Co-authored-by: Valerii Zapodovnikov > Signed-off-by: Diederick Niehorster > --- > libavdevice/dshow.c | 255 +++- > 1 file changed, 254 insertions(+), 1 deletion(-) > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > index fa3a06c077..4ad6ae102c 100644 > --- a/libavdevice/dshow.c > +++ b/libavdevice/dshow.c > @@ -29,6 +29,31 @@ > #include "libavcodec/raw.h" > #include "objidl.h" > #include "shlwapi.h" > +// NB: technically, we should include dxva.h and use > +// DXVA_ExtendedFormat, but that type is not defined in > +// the MinGW headers. The DXVA2_ExtendedFormat and the > +// contents of its fields is identical to > +// DXVA_ExtendedFormat (see > https://docs.microsoft.com/en-us/windows/win32/medfound/extended-color-information#color-space-in-media-types) > +// and is provided by MinGW as well, so we use that > +// instead. NB also that per the Microsoft docs, the > +// lowest 8 bits of the structure, i.e. the SampleFormat > +// field, contain AMCONTROL_xxx flags instead of sample > +// format information, and should thus not be used. > +// NB further that various values in the structure's > +// fields (e.g. BT.2020 color space) are not provided > +// for either of the DXVA structs, but are provided in > +// the flags of the corresponding fields of Media Foundation. > +// These may be provided by DirectShow devices (e.g. LAVFilters > +// does so). So we use those values here too (the equivalence is > +// indicated by Microsoft example code: > https://docs.microsoft.com/en-us/windows/win32/api/dxva2api/ns-dxva2api-dxva2_videodesc) > +typedef DWORD D3DFORMAT;// dxva2api.h header needs these types defined > before include apparently in WinSDK (not MinGW). > +typedef DWORD D3DPOOL; > +#include "dxva2api.h" > + > +#ifndef AMCONTROL_COLORINFO_PRESENT > +// not defined in some versions of MinGW's dvdmedia.h > +# define AMCONTROL_COLORINFO_PRESENT 0x0080 // if set, indicates DXVA > color info is present in the upper (24) bits of the dwControlFlags > +#endif > > > static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD biBitCount) > @@ -54,6 +79,192 @@ static enum AVPixelFormat dshow_pixfmt(DWORD > biCompression, WORD biBitCount) > return avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), > biCompression); // all others > } > > +static enum AVColorRange dshow_color_range(DXVA2_ExtendedFormat *fmt_info) > +{ > +switch (fmt_info->NominalRange) > +{ > +case DXVA2_NominalRange_Unknown: > +return AVCOL_RANGE_UNSPECIFIED; > +case DXVA2_NominalRange_Normal: // equal to DXVA2_NominalRange_0_255 > +return AVCOL_RANGE_JPEG; > +case DXVA2_NominalRange_Wide: // equal to DXVA2_NominalRange_16_235 > +return AVCOL_RANGE_MPEG; > +case DXVA2_NominalRange_48_208: > +// not an ffmpeg color range > +return AVCOL_RANGE_UNSPECIFIED; > + > +// values from MediaFoundation SDK (mfobjects.h) > +case 4: // MFNominalRange_64_127 > +// not an ffmpeg color range > +return AVCOL_RANGE_UNSPECIFIED; > + > +default: > +return DXVA2_NominalRange_Unknown; > +} > +} > + > +static enum AVColorSpace dshow_color_space(DXVA2_ExtendedFormat *fmt_info) > +{ > +enum AVColorSpace ret = AVCOL_SPC_UNSPECIFIED; > + > +switch (fmt_info->VideoTransferMatrix) > +{ > +case DXVA2_VideoTransferMatrix_BT709: > +ret = AVCOL_SPC_BT709; > +break; > +case DXVA2_VideoTransferMatrix_BT601: > +ret = AVCOL_SPC_BT470BG; > +break; > +case DXVA2_VideoTransferMatrix_SMPTE240M: > +ret = AVCOL_SPC_SMPTE240M; > +break; > + > +// values from MediaFoundation SDK (mfobjects.h) > +case 4: // MFVideoTransferMatrix_BT2020_10 > +case 5: // MFVideoTransferMatrix_BT2020_12 > +if (fmt_info->VideoTransferFunction==12)// > MFVideoTransFunc_2020_const > +ret = AVCOL_SPC_BT2020_CL; > +else > +ret = AVCOL_SPC_BT2020_NCL; > +break; > +} > + > +if (ret == AVCOL_SPC_UNSPECIFIED) > +{ > +// if color space not known from transfer matrix, > +// fall back to using primaries to guess color space > +switch (fmt_info->VideoPrimaries) > +{ > +case DXVA2_VideoPrimaries_BT709: > +ret = AVCOL_SPC_BT709; > +break; > +case DXVA2_VideoPrimaries_BT470_2_SysM: > +ret = AVCOL_SPC_FCC; > +break; > +case DXVA2_VideoPrimaries_BT470_2_SysBG: > +case DXVA2_VideoPrimaries_EBU3213: // this is PAL > +ret = AVCOL_SPC_BT470BG; > +break; > +case DXVA2_VideoPrimaries_SMPTE170M: > +case DXVA2_VideoP
Re: [FFmpeg-devel] [PATCH v5 13/13] avdevice/dshow: select format with extended color info
Diederick Niehorster: > Some DirectShow devices (Logitech C920 webcam) expose each DirectShow > format they support twice, once without and once with extended color > information. During format selection, both match, this patch ensures > that the format with extended color information is selected if it is > available, else it falls back to a matching format without such > information. This also necessitated a new code path taken for default > formats of a device (when user didn't request any specific video size, > etc), because the default format may be one without extended color > information when a twin with extended color information is also > available. Getting the extended color information when available is > important as it allows setting the color space, range, primaries, > transfer characteristics and chroma location of the stream provided by > dshow, enabling users to get more correct color automatically out of > their device. > > Closes: #9271 > > Signed-off-by: Diederick Niehorster > --- > libavdevice/dshow.c | 469 +++- > 1 file changed, 338 insertions(+), 131 deletions(-) > [...] > @@ -1342,6 +1556,7 @@ dshow_add_device(AVFormatContext *avctx, > AM_MEDIA_TYPE type; > AVCodecParameters *par; > AVStream *st; > +struct dshow_format_info *fmt_info = NULL; > int ret = AVERROR(EIO); > > type.pbFormat = NULL; > @@ -1356,12 +1571,14 @@ dshow_add_device(AVFormatContext *avctx, > ctx->capture_filter[devtype]->stream_index = st->index; > > ff_dshow_pin_ConnectionMediaType(ctx->capture_pin[devtype], &type); > +fmt_info = dshow_get_format_info(&type); > +if (!fmt_info) > +goto error; It is dangerous to goto error without setting ret to an error; it might work now due to the ret = AVERROR(EIO) initialization, but it is nevertheless dangerous. > > par = st->codecpar; > if (devtype == VideoDevice) { > BITMAPINFOHEADER *bih = NULL; > AVRational time_base; > -DXVA2_ExtendedFormat *extended_format_info = NULL; > > if (IsEqualGUID(&type.formattype, &FORMAT_VideoInfo)) { > VIDEOINFOHEADER *v = (void *) type.pbFormat; > @@ -1371,8 +1588,6 @@ dshow_add_device(AVFormatContext *avctx, > VIDEOINFOHEADER2 *v = (void *) type.pbFormat; > time_base = (AVRational) { v->AvgTimePerFrame, 1000 }; > bih = &v->bmiHeader; > -if (v->dwControlFlags & AMCONTROL_COLORINFO_PRESENT) > -extended_format_info = (DXVA2_ExtendedFormat *) > &v->dwControlFlags; > } > if (!bih) { > av_log(avctx, AV_LOG_ERROR, "Could not get media type.\n"); > @@ -1383,33 +1598,21 @@ dshow_add_device(AVFormatContext *avctx, > st->r_frame_rate = av_inv_q(time_base); > > par->codec_type = AVMEDIA_TYPE_VIDEO; > -par->width = bih->biWidth; > -par->height = bih->biHeight; > +par->width = fmt_info->width; > +par->height = fmt_info->height; > par->codec_tag = bih->biCompression; > -par->format = dshow_pixfmt(bih->biCompression, bih->biBitCount); > +par->format = fmt_info->pix_fmt; > if (bih->biCompression == MKTAG('H', 'D', 'Y', 'C')) { > av_log(avctx, AV_LOG_DEBUG, "attempt to use full range for > HDYC...\n"); > par->color_range = AVCOL_RANGE_MPEG; // just in case it needs > this... > } > -if (extended_format_info) { > -par->color_range = dshow_color_range(extended_format_info); > -par->color_space = dshow_color_space(extended_format_info); > -par->color_primaries = > dshow_color_primaries(extended_format_info); > -par->color_trc = dshow_color_trc(extended_format_info); > -par->chroma_location = dshow_chroma_loc(extended_format_info); > -} > -if (par->format == AV_PIX_FMT_NONE) { > -const AVCodecTag *const tags[] = { > avformat_get_riff_video_tags(), NULL }; > -par->codec_id = av_codec_get_id(tags, bih->biCompression); > -if (par->codec_id == AV_CODEC_ID_NONE) { > -av_log(avctx, AV_LOG_ERROR, "Unknown compression type. " > - "Please report type 0x%X.\n", (int) > bih->biCompression); > -ret = AVERROR_PATCHWELCOME; > -goto error; > -} > -par->bits_per_coded_sample = bih->biBitCount; > -} else { > -par->codec_id = AV_CODEC_ID_RAWVIDEO; > +par->color_range = fmt_info->col_range; > +par->color_space = fmt_info->col_space; > +par->color_primaries = fmt_info->col_prim; > +par->color_trc = fmt_info->col_trc; > +par->chroma_location = fmt_info->chroma_loc; > +par->codec_id = fmt_info->codec_id; > +if (par->codec_id == AV_CODEC_ID_RAWVIDEO) { > if (bi
Re: [FFmpeg-devel] [PATCH] libavcodec/pthread_framec: remove duplicate pointers
Ping for this patch. -Yu Yang > 2021年12月15日 上午10:17,Yu Yang 写道: > > From: Yu Yang > > '*src' and '*avctx' point to the same memory. It is enough to keep one of > them. > > Signed-off-by: Yu Yang > --- > libavcodec/pthread_frame.c | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c > index 73b1b7d7d9..98f88f7732 100644 > --- a/libavcodec/pthread_frame.c > +++ b/libavcodec/pthread_frame.c > @@ -765,14 +765,14 @@ void ff_frame_thread_free(AVCodecContext *avctx, int > thread_count) > > static av_cold int init_thread(PerThreadContext *p, int *threads_to_free, >FrameThreadContext *fctx, AVCodecContext > *avctx, > - AVCodecContext *src, const AVCodec *codec, > int first) > +const AVCodec *codec, int first) > { > AVCodecContext *copy; > int err; > > atomic_init(&p->state, STATE_INPUT_READY); > > -copy = av_memdup(src, sizeof(*src)); > +copy = av_memdup(avctx, sizeof(*avctx)); > if (!copy) > return AVERROR(ENOMEM); > copy->priv_data = NULL; > @@ -784,7 +784,7 @@ static av_cold int init_thread(PerThreadContext *p, int > *threads_to_free, > p->parent = fctx; > p->avctx = copy; > > -copy->internal = av_memdup(src->internal, sizeof(*src->internal)); > +copy->internal = av_memdup(avctx->internal, sizeof(*avctx->internal)); > if (!copy->internal) > return AVERROR(ENOMEM); > copy->internal->thread_ctx = p; > @@ -798,7 +798,7 @@ static av_cold int init_thread(PerThreadContext *p, int > *threads_to_free, > > if (codec->priv_class) { > *(const AVClass **)copy->priv_data = codec->priv_class; > -err = av_opt_copy(copy->priv_data, src->priv_data); > +err = av_opt_copy(copy->priv_data, avctx->priv_data); > if (err < 0) > return err; > } > @@ -843,7 +843,6 @@ int ff_frame_thread_init(AVCodecContext *avctx) > { > int thread_count = avctx->thread_count; > const AVCodec *codec = avctx->codec; > -AVCodecContext *src = avctx; > FrameThreadContext *fctx; > int err, i = 0; > > @@ -876,7 +875,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) > fctx->delaying = 1; > > if (codec->type == AVMEDIA_TYPE_VIDEO) > -avctx->delay = src->thread_count - 1; > +avctx->delay = avctx->thread_count - 1; > > fctx->threads = av_calloc(thread_count, sizeof(*fctx->threads)); > if (!fctx->threads) { > @@ -888,7 +887,7 @@ int ff_frame_thread_init(AVCodecContext *avctx) > PerThreadContext *p = &fctx->threads[i]; > int first = !i; > > -err = init_thread(p, &i, fctx, avctx, src, codec, first); > +err = init_thread(p, &i, fctx, avctx, codec, first); > if (err < 0) > goto error; > } > -- > 2.33.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 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] MAINTAINERS: Add Haihao Xiang for qsv
On Wed, 2021-12-15 at 16:24 +0800, Zhong Li wrote: > Eoff, Ullysses A 于2021年12月15日周三 00:52写道: > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > > > index dcac46003e..39ce91b755 100644 > > > > > > --- a/MAINTAINERS > > > > > > +++ b/MAINTAINERS > > > > > > @@ -226,7 +226,7 @@ Codecs: > > > > > >ptx.c Ivo van Poorten > > > > > >qcelp*Reynaldo H. Verdejo > > > > > > Pinochet > > > > > >qdm2.c, qdm2data.hRoberto Togni > > > > > > - qsv* Mark Thompson, Zhong Li > > > > > > + qsv* Mark Thompson, Zhong Li, > > > > > > Haihao Xiang > > > > > >qtrle.c Mike Melanson > > > > > >ra144.c, ra144.h, ra288.c, ra288.hRoberto Togni > > > > > >resample2.c Michael Niedermayer > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > ___ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > [UAE] +1, LGTM > > > > > > > > > > > > > > > > > > *bump* > > > > > > > > > > CC Mark and LiZhong > > > > CC LiZhong again with [hopefully] correct email contact. > > (previous email contact bounced) > > Will apply this patch during one week if no objection. ( And will > merge some other qsv patches from haihao too) Thanks Zhong, Artie, Softworkz etc. I pinged Zhong and Mark via email a few months ago for co-maintaining QSV voluntarily and will continue to offer efforts to improve QSV, make it more stable and useful to QSV user. BRs Haihao ___ 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] qsvenc_hevc: Enable look ahead with ExtBRC
On Sun, 2021-12-19 at 23:55 +0800, Zhong Li wrote: > Xiang, Haihao 于2021年12月6日周一 11:09写道: > > > --- a/libavcodec/qsvenc_hevc.c > > > +++ b/libavcodec/qsvenc_hevc.c > > > @@ -248,6 +248,7 @@ static const AVOption options[] = { > > > { "tile_rows", "Number of rows for tiled > > > encoding", OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 }, > > > 0, > > > UINT16_MAX, VE }, > > > { "recovery_point_sei", "Insert recovery point SEI > > > messages", OFFSET(qsv.recovery_point_sei), AV_OPT_TYPE_INT, { > > > .i64 > > > = -1 }, -1, 1, VE }, > > > { "aud", "Insert the Access Unit Delimiter NAL", OFFSET(qsv.aud), > > > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, VE}, > > > +{ "look_ahead_depth", "Depth of look ahead in number frames", > > > OFFSET(qsv.look_ahead_depth), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 100, VE }, > > > > > > { NULL }, > > > }; > > > > Any comment for this patch ? Could someone help to merge this patch if no > > objection ? > > How to verify this function? is it depended on a special version of > MSDK && vaapi-driver? > I tried to enable extbrc and disable/enable look_ahead_depth (set to > zero/20) for hevc_qsv but found there are no difference of the encoded > files. AFAIK this function depends on your MSDK runtime. There is no impact when user turns on this option but the MSDK runtime doesn't support it. Thanks Haihao ___ 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] MAINTAINERS: Add Haihao Xiang for vaapi
On Wed, 2021-12-15 at 15:32 +0100, Michael Niedermayer wrote: > On Tue, Dec 14, 2021 at 10:31:40PM +, Soft Works wrote: > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Michael > > > Niedermayer > > > Sent: Tuesday, December 14, 2021 10:40 PM > > > To: FFmpeg development discussions and patches > > > Subject: Re: [FFmpeg-devel] [PATCH] MAINTAINERS: Add Haihao Xiang for > > > vaapi > > > > > > On Tue, Dec 14, 2021 at 12:16:16PM -0800, Philip Langdale wrote: > > > > On Tue, 14 Dec 2021 16:39:40 + > > > > "Eoff, Ullysses A" wrote: > > > > > > > > > I have not seen any objections. > > > > > I just added Mark to CC using their email found on this ML. > > > > > Unfortunately, we don't have current email contact for > > > > > Gwenole (whom has not worked on ffmpeg for ~6 years). > > > > > Who else can make the approval, aside from the inactive people > > > > > previously listed? > > > > > > > > Are people reluctant to approve? > > > > > > I dont think so, iam just extra carefull as someone on IRC noticed that > > > "everyone and their grandmother who's worked for intel's got push access" > > > > > > There where also some complaints about code quality/cleanlyness > > > toward intels contributions > > > > That's true. There are differences, and my expressed support is > > specifically tied to the person, not about having just "somebody". > > > > I have my reservations about too early adoption of oneVPL and I think > > the patchset "Cleanup QSV Filters.." is doing too many things at once, > > but well - being a maintainer still doesn't mean that one could merge > > anything without consent. > > > > But the current situation, like having two or three people working full-time > > on the subject being depending on somebody who isn't following progress > > and unable assess, evaluate and test proposed changes is quite awkward. > > yes > > > > > > > So i just wanted to make sure there are no objections (iam not conciously > > > aware of any objections to these MAINTAINER additions ATM) > > > > > > > > > > I'll give it my approval to have on > > > > the record. If the old maintainers want to emerge from the woodwork and > > > > object after the fact, they are welcome to do so, but we can't just sit > > > > around indefinitely. > > > > I'm actually wondering, whether registered maintainers that do not > > respond anymore to contributions for a certain amount of time, > > shouldn't be unregistered automatically at some point? > > If they never react they should be removed when a new volunteer is added. > they should be informed about that when possible though. > Also when there is noone else, then listing the last maintainer even if > (s)he is mostly inactive still can make sense as it gives at least some point > of contact Thanks all of you for your kink support. I'm glad to maintain vaapi plugin, will offer efforts to improve / enhance vaapi plugins and make it more useful to vaapi user. I'd appreciate it if someone could make the approval and apply this patch. Thanks Haihao ___ 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".