Re: [FFmpeg-devel] [PATCH v2 2/5] avutil/hwcontext_qsv: derive QSV frames to D3D11VA frames
> -Original Message-
> From: ffmpeg-devel On Behalf Of Tong
> Wu
> Sent: Friday, April 29, 2022 12:45 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Tong Wu
> Subject: [FFmpeg-devel] [PATCH v2 2/5] avutil/hwcontext_qsv: derive
> QSV frames to D3D11VA frames
>
> Fixes:
> $ ffmpeg.exe -y -hwaccel qsv -init_hw_device d3d11va=d3d11 \
> -init_hw_device qsv=qsv@d3d11 -c:v h264_qsv -i input.h264 \
> -vf "hwmap=derive_device=d3d11va,format=d3d11" -f null -
>
> Signed-off-by: Tong Wu
> ---
> libavutil/hwcontext_qsv.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> index 66c0e38955..d9d05e936a 100644
> --- a/libavutil/hwcontext_qsv.c
> +++ b/libavutil/hwcontext_qsv.c
> @@ -808,12 +808,23 @@ static int
> qsv_frames_derive_from(AVHWFramesContext *dst_ctx,
> #if CONFIG_D3D11VA
> case AV_HWDEVICE_TYPE_D3D11VA:
> {
> +dst_ctx->initial_pool_size = src_ctx->initial_pool_size;
> AVD3D11VAFramesContext *dst_hwctx = dst_ctx->hwctx;
> -mfxHDLPair *pair = (mfxHDLPair*)src_hwctx-
> >surfaces[i].Data.MemId;
> -dst_hwctx->texture = (ID3D11Texture2D*)pair->first;
> +dst_hwctx->texture_infos = av_calloc(src_hwctx-
> >nb_surfaces,
> + sizeof(*dst_hwctx-
> >texture_infos));
> if (src_hwctx->frame_type & MFX_MEMTYPE_SHARED_RESOURCE)
> dst_hwctx->MiscFlags = D3D11_RESOURCE_MISC_SHARED;
> dst_hwctx->BindFlags =
> qsv_get_d3d11va_bind_flags(src_hwctx->frame_type);
> +for (i = 0; i < src_hwctx->nb_surfaces; i++) {
> +mfxHDLPair* pair = (mfxHDLPair*)src_hwctx-
> >surfaces[i].Data.MemId;
> +dst_hwctx->texture_infos[i].texture =
> (ID3D11Texture2D*)pair->first;
> +if (dst_hwctx->BindFlags & D3D11_BIND_RENDER_TARGET)
> {
> +dst_hwctx->texture_infos[i].index = 0;
> +}
> +else {
> +dst_hwctx->texture_infos[i].index =
> (intptr_t)pair->second;
> +}
> +}
> }
> break;
> #endif
> --
LGTM.
This has always been incomplete and untested since my original patchset.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/5] avutil/hwcontext_d3d11va: fix the uninitialized texture bindflag
> -Original Message- > From: ffmpeg-devel On Behalf Of > Hendrik Leppkes > Sent: Saturday, April 30, 2022 12:02 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v2 1/5] avutil/hwcontext_d3d11va: > fix the uninitialized texture bindflag > > On Fri, Apr 29, 2022 at 12:45 PM Tong Wu > wrote: > > > > When uploading rawvideos using d3d11va hardware framecontext, the > bindflag > > is not initialized and will cause creating texture failure. Now fix > it, > > assign it the value of D3D11_BIND_RENDER_TARGET. > > > > As with similar fixes of this nature, this implicit behavior to fix > one particular bug does not seem fitting inside the hwcontext itself. > There can be a large list of usages of the hwcontext that all require > different BindFlags, but we can only define one default - why this one > specifically? I agree that this change is not ideal. On one side, it is "safe" in a way that a texture is practically unusable for video processing without having at least one of the flags (decoder, encoder or render_target), so this wouldn't "hurt" anybody. > So rather: > > Where is the context created? Looking at the command line in the commit message, this is about standalone D3D11 context creation. > Why is a required flag not set there? That would be better, because > that knows what flags it needs. There doesn't really exist an appropriate "there". I see two options 1. Add a generic internal device creation parameter to the dictionary in ffmpeg_hw.c like "standalone=1" (for all devices created via init_hw_device) Some time ago, I had another case where I thought this could be useful. Then, this could be used in d3d11va_device_create() to set an internal field 'default_bindflags' which would be used as condition in d3d11va_frames_init. The situation would remain similar though, as that when the device is used by a decoder (which sets the decoder flag) this needs to override the default. 2. Use a device parameter specific to the D3D11 hwcontext This would need to be specified in the command line. Everything else like in #1 What do you think? Best regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of nil- > admir...@mailo.com > Sent: Friday, April 29, 2022 8:52 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > > A code change for which no use case exists and does not provide > > any benefit is not relevant. That's my point. > > You've deleted me saying > > >> You're talking as if MAX_PATH limited library loader is self- > evidently > >> superior, and it is the loader that has no such limitation that has > to justify > >> its existence. As far as I'm concerned it just the other way > around. > > and now claim that code change is irrelevant. Yes, because there is no realistic case in which ffmpeg will need to load any dll from a long path. I've asked you multiple times to give an example. > > Imagine, you are creating a software (no matter whether you're big > or small, > > open or closed source, targeting business or home users, using a > custom or > > public built ffmpeg) and you bundle ffmpeg.exe with your software > like many > > are doing. Now, re-read my comments, maybe it will make more sense > to you. > > They do not. Customer ask for long path support and gets two > responses: > > 1. Enable long path support via registry once, and never care about it > again. > 2. Convert path to absolute and prefix it with \\?\ every time you use > our software. > > I don't see how second workaround can be any good at all. With the > first option, > customers can at least pressure Microsoft into making it a default So, ffmpeg should implement something that doesn't work without a registry change in order to "pressure Microsoft" to change it at some point in the future? > , if > registry tweak > is too much a hassle for them; with the second they're stuck with > workarounds—forever. I apologize when I have created the impression that ffmpeg should be left as is and users should continue adding the long path prefix themselves. I'm not opposed to adding support for long paths, I'm just opposed to the method. Implementing long path support by adding the prefix is not a "workaround". Those kinds of paths are part of OS functionality since Windows NT and will still work in Windows 20 or whatever may come. What IS a workaround is having to add the prefix manually to paths when using ffmpeg. You wrote > with the second they're stuck with > workarounds—forever. But it's just the other way round: Because with the first, you cannot rely that long paths are working, you would be stuck needing to continue prefixing paths manually "forever". (to also cover cases where it's not working) > > ffmpeg is already working pretty well in handling long file paths > (also with > > Unicode characters) when pre-fixing paths with \\?\ > > It handles them most of the time, but not always. I already mentioned > that the code from > https://ffmpeg.org/pipermail/ffmpeg-devel/2022-April/295568.html > explicitly converts all backslashes into forward slashes for unknown > reasons, > and that code will not work with \\?\ paths because //?/ is not a > valid prefix. IIRC this code is handling code paths relative to the executable path, and the executable doesn't run from a long path anyway. Nonetheless it might make sense to adjust that slash replacement for Windows. Same like you, I don't know why it has been added. > > This patchset does not provide reliable behavior. > > Actually it does. > > > This way, you won't be able to use long paths with ffmpeg within the > next 5-8 years at minimum, > > Long paths can be used since August, 2 2016. Some ~6 years have > already passed. You know what I meant: it's not activated by default. > > because even in the latest versions of Windows 11, this is not > enabled by > > default in the operating system. > > FFmpeg isn't bundled by default either. And not everyone has rights to > download and run arbitrary software on their machines. That's right, but that's not a question that matters because that's not in the hands of ffmpeg. What is in the hands of ffmpeg though, is requiring a change which needs administrative privileges or not. > > What is the value of adding a capability where it will be a lottery > > game whether it will work or not on each system? > > Windows 10 > 1607 + registry key = it works. Otherwise it doesn't. > There is no lottery. I don't want to argue about the term lottery. What I'm saying is that a solution is preferable which doesn't have an "Otherwise it doesn't" part. Best regards softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS
> -Original Message- > From: ffmpeg-devel On Behalf Of > Michael Niedermayer > Sent: Monday, April 25, 2022 8:40 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS > > On Mon, Apr 25, 2022 at 08:04:25PM +0200, Jean-Baptiste Kempf wrote: > > On Mon, 25 Apr 2022, at 19:19, Michael Niedermayer wrote: > > > On Mon, Apr 25, 2022 at 01:51:26PM +0200, Jean-Baptiste Kempf > wrote: > > >> On Sat, 23 Apr 2022, at 18:36, Michael Niedermayer wrote: > > >> > Do people prefer that 5.0 becomes LTS or the next (5.1) ? > > >> > Or something else ? > > >> > > >> My understanding of the consensus was; > > >> - 5.0 in Dec/Jan > > >> - 5.1 in Jul with API additions, but no ABI/behavior breakage > > > > > > yes > > > > OK. > > > > >> - 6.0 in Dec 22 with ABI/API/behavior breakage and while 5.1 > becomes LTS > > > > > > we could give 5.1 an LTS "tag" when its released already > > > > I would mention it, tbh. > > > > > also thers the possibility that by december we have nothing that > really > > > benefits from a ABI/API/behavior breakage. > > > If that happens people might prefer 5.2 over 6.0 i dont know. > > > > I think it would be clearer to accustom people that we have one big > potential change every year, at the same date. > > thats not a bad idea. Still bumping the sonames of the libs when there > is > no reason is not really nice. I dont know if we will have a reason in > december I think, the most valid reason to increase a version number is the publishing of a release :-) Totally crazy concept: ffmpeg 6.0 libavutil.so.6.0.100 libavcodec.so.6.0.100 libavfilter.so.6.0.100 libavformat.so.6.0.100 ... ffmpeg 6.1 libavutil.so.6.1.100 libavcodec.so.6.1.100 libavfilter.so.6.1.100 libavformat.so.6.1.100 ... Best, softworkz (yes, I mean no cross-version compatibility) ___ 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 v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of > Martin Storsjö > Sent: Monday, April 25, 2022 3:02 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > On Mon, 25 Apr 2022, Hendrik Leppkes wrote: > > > On Mon, Apr 25, 2022 at 1:12 PM Soft Works > wrote: > >> > >> From my point of view: > >> ffmpeg is already working pretty well in handling long file paths > (also with > >> Unicode characters) when pre-fixing paths with \\?\, and this is > working > >> on all Windows versions without all the caveats, requirements and > conditions > >> that I mentioned. > > > > "We have already worked around this problem in our deployment, > > therefore there is no need to try to improve it" is surely not a > very > > strong argument. > > > > Will your work-around continue to work? Yes. > > Will the changes actually impact anyone negatively? No known case is > > documented, here or otherwise. > > Will this change objectively improve the operation of ffmpeg on > > Windows? Maybe not for everyone (yet), but certainly it'll allow it > to > > do so in controlled environments. > > > > I'm not seeing a good argument here to generally block the patch on, > > as this entire thread boils down to .. what? Fear of change? > > Unless you can demonstrate an actual problem resulting from applying > > this patch, this line of arguments seems not very productive. > > +1, I agree here. > > Asking users to manually use \\?\ paths isn't something we should > recommend as solution. > > Internally prepending \\?\ when necessary could work (and would also > be a > possible fix, which at least would fix everything that goes through > avio), > but that's clearly much more risky than just adding a mostly-noop > manifest. (And that only works for absolute paths; it requires some > path > munging logic to be able to do that.) I wouldn't mind going that way > too, > but the current patchset seems risk free to me. > > FWIW, LLVM does something like that [1] - before opening files, it > checks > if a path seems to be too long, and if it is, it converts it into an > absolute path and adds such a prefix. But that's clearly more risky > and > more nontrivial than this patchset. I see it the other way round. This patchset does not provide reliable behavior. This way, you won't be able to use long paths with ffmpeg within the next 5-8 years at minimum, because even in the latest versions of Windows 11, this is not enabled by default in the operating system. What is the value of adding a capability where it will be a lottery game whether it will work or not on each system? Best regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of > Hendrik Leppkes > Sent: Monday, April 25, 2022 2:52 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > On Mon, Apr 25, 2022 at 1:12 PM Soft Works > wrote: > > > > From my point of view: > > ffmpeg is already working pretty well in handling long file paths > (also with > > Unicode characters) when pre-fixing paths with \\?\, and this is > working > > on all Windows versions without all the caveats, requirements and > conditions > > that I mentioned. > > "We have already worked around this problem in our deployment, > therefore there is no need to try to improve it" is surely not a very > strong argument. > > Will your work-around continue to work? Yes. > Will the changes actually impact anyone negatively? No known case is > documented, here or otherwise. You may want to read this: https://docs.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page > Will this change objectively improve the operation of ffmpeg on > Windows? Maybe not for everyone (yet), but certainly it'll allow it to > do so in controlled environments. > > I'm not seeing a good argument here to generally block the patch on, > as this entire thread boils down to .. what? Fear of change? Pardon? Seems you missed my point (probably you haven't followed the full conversation). What I'm saying is that prepending the long path prefix is the better way for supporting long paths and I mentioned our experience with it only to confirm that it's working well. The .NET/corefx runtime uses the prefix method internally, rclone is using it, the Java runtime is using it, just to name a few examples. Best regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of nil- > admir...@mailo.com > Sent: Monday, April 25, 2022 11:52 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > > Again, you have deleted my asking for an example scenario > > and which library would need to be loaded from a long path. > > Because I don't think that an example would be relevant. A code change for which no use case exists and does not provide any benefit is not relevant. That's my point. > > For the longpaths attribute, you could surely argue that it's "just" > > that you don't know whether it will work or not. > > But forcing a different code page for a process _IS_ a fundamental > > alteration. > > Which is necessary for compatibility with AviSynth, and commit message > says exactly that: https://ffmpeg.org/pipermail/ffmpeg-devel/2022- > April/295572.html. Yes, I understand. You want to make a fundamental change to ffmpeg because of AviSynth. > > I'm restoring the line of your own question which you have deleted: > > > > > > Is it a big deal to change a registry and reboot? > > My response to that was: > > ... > > Really? I answer your question and then you delete your question > > and ask what my answer would have to do with the patchset? > > Sorry, I should've directly asked what: > > > 3. A registry key or group policy needs to be set on Windows to > enable this > > ´ (in both cases, administrative permission/UAC is required to set > it) > > 4. Even when registry key or group policy is set, it might still be > pending > >a reboot > > has to do with this patchset, instead of asking a rhetorical question > of whether > it's a big deal or not. Imagine, you are creating a software (no matter whether you're big or small, open or closed source, targeting business or home users, using a custom or public built ffmpeg) and you bundle ffmpeg.exe with your software like many are doing. Now, re-read my comments, maybe it will make more sense to you. If not - I'm in no way authoritative for ffmpeg, others may have different opinions. From my point of view: ffmpeg is already working pretty well in handling long file paths (also with Unicode characters) when pre-fixing paths with \\?\, and this is working on all Windows versions without all the caveats, requirements and conditions that I mentioned. Best regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of nil- > admir...@mailo.com > Sent: Monday, April 25, 2022 11:04 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > > You normally don't load libraries from such longs paths. > > And? 80% of FFmpeg functionality is normally not used. Again, you have deleted my asking for an example scenario and which library would need to be loaded from a long path. > > But file IO is a fundamental feature where a common and predictable > > behavior is crucial. > > You're talking as if the manifest change somehow broke or > fundamentally altered > file IO, which it clearly did not. For the longpaths attribute, you could surely argue that it's "just" that you don't know whether it will work or not. But forcing a different code page for a process _IS_ a fundamental alteration. I'm restoring the line of your own question which you have deleted: > > > > Is it a big deal to change a registry and reboot? My response to that was: > > Yes it is. Especially when you don't know that it's needed or > whether > > it's needed or when it is needed. > > Also it requires administrative permissions which not everybody has. > > Further it's a serious change to OS behavior and you cannot expect > that > > all users are educated enough for being able to make this decision > > and be confident that it won't have any unexpected side effects. > > What does all of that have to do with these patches? Really? I answer your question and then you delete your question and ask what my answer would have to do with the patchset? sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Monday, April 25, 2022 10:01 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS > > On Mon, Apr 25, 2022 at 12:48 AM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Michael Niedermayer > > > Sent: Saturday, April 23, 2022 6:36 PM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS > > > > > > Hi all > > > > > > Do people prefer that 5.0 becomes LTS or the next (5.1) ? > > > Or something else ? > > > > @Anton - I'm wondering how your multi-threading transformations > > to ffmpeg.c would align with the release schedule? > > > > I think an LTS release should be either before or after the > > series of changes you are planning. It would be unfortunate > > when 5.1 would be somewhere in the middle of those changes > > and become LTS. > > (as you said it could take a year in the worst case) > > > > > I'm still expecting info that you said to provide. That "information" is about explaining why I said that it's difficult to debug filtering in ffmpeg. I haven't forgot about and I'll get to it shortly. As I mentioned before, I'm concerned about the transitioning process but not opposed to the end result. My suggestion was to retain the current code flow as an option, but nobody else found it useful - that's ok, I have no intentions to insist. Besides that, from my personal pov: the faster it can be driven to completion, the better, so there's just a "before-state" (the current state) and a "completion state" to care about. Having a release in-between would create an additional state to deal with, and that's what I would find unfortunate. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS
> -Original Message- > From: ffmpeg-devel On Behalf Of > Michael Niedermayer > Sent: Saturday, April 23, 2022 6:36 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: [FFmpeg-devel] FFmpeg 5.0 LTS vs 5.1 LTS > > Hi all > > Do people prefer that 5.0 becomes LTS or the next (5.1) ? > Or something else ? @Anton - I'm wondering how your multi-threading transformations to ffmpeg.c would align with the release schedule? I think an LTS release should be either before or after the series of changes you are planning. It would be unfortunate when 5.1 would be somewhere in the middle of those changes and become LTS. (as you said it could take a year in the worst case) Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of nil- > admir...@mailo.com > Sent: Sunday, April 24, 2022 2:08 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v11 1/6] > libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and > utf8toansi > > > 1. Patch 3/6 - Replace LoadLibraryExA with LoadLibraryExW > > What's the point in making changes to library loading? What does it > fix? > > > > From the commit https://ffmpeg.org/pipermail/ffmpeg-devel/2022- > April/295571.html: > > ... the path length limitation is only lifted for file APIs that pass > paths as wchar_t... You normally don't load libraries from such longs paths. As I had written previously (but stripped off by you): Could you please give an example of a situation that this is supposed to fix? Which dll do you think would need to be loaded from a long path? > > 2. Patches 5/6 and 6/6 - Add Fusion Manifest > > ... > > Both of these manifest attributes are affecting the runtime behavior > of > > an application running on Windows - but only starting from a certain > > OS version. > > ... you would need to check the operating system version > > before using to make sure that you are providing parameters in the > "right" > > way - I'm not sure whether that would make much sense. > > > The same can be said about e.g. DirectX 11 hardware acceleration, > which is available > only starting with Windows 7; and you have to check OS version before > providing > parameters that enable it. Does not mean implementing that feature > made no sense, > since it made FFmpeg inconsistent with previous versions of Windows. > Consistency > > with the greatest common denominator does not make much sense. Your example is making a comparison that is not quite valid IMO. The use of DirectX 11 is a very specific feature and that surely involves checking of availability. But file IO is a fundamental feature where a common and predictable behavior is crucial. > > 3. All Patches x/6 - Remove MAX_PATH limit > > The punch line sounds compelling. But how is the current situation > and > > what exactly would be the benefit? > > All patches with that line replace MAX_PATH-sized buffers with > dynamically-allocated > > buffers of potentially arbitrary size. If application continues to use > fixed-size buffers, > > it's not long path aware, regardless of the manifest. That's true, but the file IO in ffmpeg does not use MAX_PATH buffers, so it's only about those few specific cases: Patch 2/6: Removing the fixed-size buffer is surely a good change, but can't you just completely remove the charset conversion and use the same file IO patterns that ffmpeg is using everywhere else? Patch 3/6: see 1. Patch 4/6: Seems to be pointless because you cannot run ffmpeg.exe from a long path anyway. Neither with cmd nor with powershell. Even if you could, it would be a pretty exotic use case. > It is neither predictable nor reliable. Please check > https://googleprojectzero.blogspot.com/2016/02/the-definitive-guide- > on-win32-to-nt.html > > for things like “I actually exploited this behaviour to create > arbitrary named pipes from the Chrome sandbox awhile back.” I know about that. But it's something that is possible now already and would be still possible after applying your patchset. So, that's not a relevant point in this regard. > > 1. Windows version needs to be >= 1903 > > (for being able to have both attributes in effect) > For all intents and purposes, versions of Windows < 1903 have EOLed. > Exceptions: > > - Windows 8.1 will EOL in January 2023, > - LTSB/LTSC branches of Windows 10 before 21H2. And the servers? And all those which are still using older versions?` > > 3. A registry key or group policy needs to be set on Windows to > enable this > > ´ (in both cases, administrative permission/UAC is required to set > it) > > 4. Even when registry key or group policy is set, it might still be > pending > > a reboot > > Is it a big deal to change a registry and reboot? Yes it is. Especially when you don't know that it's needed or whether it's needed or when it is needed. Also it requires administrative permissions which not everybody has. Further it's a serious change to OS behavior and you cannot expect that all users are educated enough for being able to make this decision and be confident that it won't have any unexpected side effects. > > On the other side, there's a risk of regressions by adding those > manifest > > attributes. > > Adding anything risks regressions, how manifest is supposed to be > different from any other patch? That's what I tried to explain. "Any other patch" makes a change after which you know that the change has been made. But adding those attributes will impose changes that will be effective under
Re: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
> -Original Message- > From: ffmpeg-devel On Behalf Of Nil > Admirari > Sent: Saturday, April 23, 2022 10:56 PM > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH v11 1/6] libavutil/wchar_filename.h: > Add whcartoutf8, wchartoansi and utf8toansi > > These functions are going to be used in libavformat/avisynth.c > and fftools/cmdutils.c to remove MAX_PATH limit. > --- Hi, thanks for submitting this patchset. I'm afraid that I hadn't found time to look into this at an earlier stage, so please apologize if one of my questions has been covered before. 1. Patch 3/6 - Replace LoadLibraryExA with LoadLibraryExW What's the point in making changes to library loading? What does it fix? Which dll cannot be loaded with the current implementation and what would be the location of the dll and the location of the exe in that case? Could you please give an example of a situation that this is supposed to fix? 2. Patches 5/6 and 6/6 - Add Fusion Manifest The manifest that you want to add includes two settings: - longPathAware effective on Windows 10, version 1607 or later - activeCodePage effective on Windows 10, version 1903 or later Both of these manifest attributes are affecting the runtime behavior of an application running on Windows - but only starting from a certain OS version. That means - effectively you would end up having an ffmpeg executable with three different kinds of behavior: 1. Win < 1607 2. Win >= 1607 && < 1903 3. Win >= 1903 An ffmpeg executable, where you can't rely on the exposed behavior being consistent and where you would need to check the operating system version before using to make sure that you are providing parameters in the "right" way - I'm not sure whether that would make much sense. 3. All Patches x/6 - Remove MAX_PATH limit The punch line sounds compelling. But how is the current situation and what exactly would be the benefit? What's important to understand is that the basic Windows file APIs are supporting long (> MAX_PATH length) for a long time already. MS has just been a bit hesitant regarding the transition and wanted to avoid breaking changes to the API. The outcome was that long paths are only supported when using a lower-level path syntax, which is as simple as prepending "\\?\" to the actual path, no matter whether drive or UNC network path. As an example, the following command runs without issue on Windows with a normal current ffmpeg build: .\ffmpeg.exe -i "\\?\U:\TestMedia\This is a very long path - This is a very long path - This is a very long path - This is a very long path - This is a very long path -\This is a very long path - This is a very long path - This is a very long path - This is a very lon\aassdd\TestMediaThis is a very long path - This is a very long path - This is a very long path - This is a very long path - This is a very long path -This is a very long path - This is a very long path - Thi - 不要抬头.ts" It's not only a path with len > MAX_PATH, the name also includes Chinese (non-ANSI) characters. All this is working in a predictable and reliable way on all common Windows versions. Recently, MS have taken additional effort in order to ease compatibility and improve platform portability, by allowing applications to use ANSI file APIs with long paths and UTF-8 charset. BUT: there are several prerequisites for this to work: 1. Windows version needs to be >= 1903 (for being able to have both attributes in effect) 2. Application needs to be compiled with and manifest file as resource 3. A registry key or group policy needs to be set on Windows to enable this ´ (in both cases, administrative permission/UAC is required to set it) 4. Even when registry key or group policy is set, it might still be pending a reboot SUMMARY From my understanding, the benefit of this patchset would be: When 1. and 2. and 3. and 4. are fulfilled (and only then) - it would be possible to use long path names in ffmpeg _without_ prefixing it with '\\?\' On the other side, there's a risk of regressions by adding those manifest attributes. I wonder whether it wouldn’t be a better idea, to simply auto-add this prefix in the ffmpeg file handling code if: - OS == Windows - len > MAX_PATH This would work on all common Windows versions and would be predictable and reliable. Best regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Thursday, April 14, 2022 12:02 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > On Wed, Apr 13, 2022 at 12:43 AM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > Paul > > > B Mahol > > > Sent: Tuesday, April 12, 2022 11:29 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > On Mon, Apr 11, 2022 at 10:58 PM Soft Works > > > > wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > From: ffmpeg-devel On Behalf > Of > > > Paul > > > > > B Mahol > > > > > Sent: Monday, April 11, 2022 10:52 PM > > > > > To: FFmpeg development discussions and patches > > > > de...@ffmpeg.org> > > > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a > threaded > > > > > architecture > > > > > > > > > > On Mon, Apr 11, 2022 at 10:10 PM Soft Works > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > -Original Message- > > > > > > > From: ffmpeg-devel On > Behalf > > > Of > > > > > > > Anton Khirnov > > > > > > > Sent: Monday, April 11, 2022 10:29 AM > > > > > > > To: FFmpeg development discussions and patches > > > > > > de...@ffmpeg.org> > > > > > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a > > > threaded > > > > > > > architecture > > > > > > > > > > > > > > Quoting Soft Works (2022-04-08 17:27:10) > > > > > > > > > Furthermore, remember that this is just the first > step. > > > There > > > > > will > > > > > > > be > > > > > > > > > further patchsets converting the other components. I > > > intend to > > > > > > > > > upstream > > > > > > > > > them gradually one after the other. Your suggestion > would > > > > > require > > > > > > > me > > > > > > > > > to > > > > > > > > > instead write the whole thing at once, fighting rebase > > > > > conflicts > > > > > > > all > > > > > > > > > the > > > > > > > > > way, and then submit it as a giant utterly > unreviewable > > > > > patchset. > > > > > > > > > > > > > > > > That's not what I meant, but anyway it's not worth > > > discussing > > > > > when > > > > > > > > it's a minority opinion. > > > > > > > > > > > > > > > > Just a practical question instead for planning purposes: > > > > > > > > > > > > > > > > Which timeframe do you expect for the whole process? > > > > > > > > When do you plan to start > > > > > > > > > > > > > > If you mean "start pushing the patches", then I intend to > do > > > that > > > > > as > > > > > > > they are reviewed and approved. I hope to send the > > > upstreamable > > > > > > > version > > > > > > > of this set this week, if nobody has strong objectsions > then I > > > > > might > > > > > > > push it after vacation, i.e. late April/early May. > > > > > > > > > > > > > > > and for how long do you think it will take until all > further > > > > > > > patchsets > > > > > > > > will be submitted/applied? > > > > > > > > > > > > > > This is very hard to estimate accurately. A pessimistic > guess > > > > > assuming > > > > > > > I > > > > > > > get stuck on every stupid thing would be end of this year,
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Nicolas George > Sent: Wednesday, April 13, 2022 11:43 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Soft Works (12022-04-12): > > I have always asked you kindly > > You have been repeatedly rude towards the people who know libavfilter > well. That treatment was exclusively reserved to yours. You had called for it. You got it. You deserved it. > On top of that, you have shown that you do not understand how > libavfilter currently works. > On top of that, you have refused to learn how libavfilter currently > works. Exactly that's one of the reasons. You keep throwing around exactly those kinds of statements without substantial content. That's purely demagogic behavior. As a long as you continue like that, you don't deserve any better responses. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Tuesday, April 12, 2022 11:29 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > On Mon, Apr 11, 2022 at 10:58 PM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > Paul > > > B Mahol > > > Sent: Monday, April 11, 2022 10:52 PM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > On Mon, Apr 11, 2022 at 10:10 PM Soft Works > > > > wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > From: ffmpeg-devel On Behalf > Of > > > > > Anton Khirnov > > > > > Sent: Monday, April 11, 2022 10:29 AM > > > > > To: FFmpeg development discussions and patches > > > > de...@ffmpeg.org> > > > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a > threaded > > > > > architecture > > > > > > > > > > Quoting Soft Works (2022-04-08 17:27:10) > > > > > > > Furthermore, remember that this is just the first step. > There > > > will > > > > > be > > > > > > > further patchsets converting the other components. I > intend to > > > > > > > upstream > > > > > > > them gradually one after the other. Your suggestion would > > > require > > > > > me > > > > > > > to > > > > > > > instead write the whole thing at once, fighting rebase > > > conflicts > > > > > all > > > > > > > the > > > > > > > way, and then submit it as a giant utterly unreviewable > > > patchset. > > > > > > > > > > > > That's not what I meant, but anyway it's not worth > discussing > > > when > > > > > > it's a minority opinion. > > > > > > > > > > > > Just a practical question instead for planning purposes: > > > > > > > > > > > > Which timeframe do you expect for the whole process? > > > > > > When do you plan to start > > > > > > > > > > If you mean "start pushing the patches", then I intend to do > that > > > as > > > > > they are reviewed and approved. I hope to send the > upstreamable > > > > > version > > > > > of this set this week, if nobody has strong objectsions then I > > > might > > > > > push it after vacation, i.e. late April/early May. > > > > > > > > > > > and for how long do you think it will take until all further > > > > > patchsets > > > > > > will be submitted/applied? > > > > > > > > > > This is very hard to estimate accurately. A pessimistic guess > > > assuming > > > > > I > > > > > get stuck on every stupid thing would be end of this year, but > I > > > hope > > > > > for things to go much faster. > > > > > > > > Thanks for the reply. I'm asking because I need to decide about > the > > > > way I'm going to proceed with the subtitle filtering patchset. > > > > > > > > I think I will have to keep and continue this in private during > this > > > > procedure as I don't have the resources to regularly adapt and > sync > > > > from my (5.0 based) working branch back to the master branch. > > > > > > > > > > > That is big waste of resource when not implementing thing > properly. > > > > From my point of view, somebody who has never given any detailed > > reviews, didn't state what exactly(!) he would consider to be > "improper" > > and never made any suggestion how the implementation would need to > > be changed to become "proper" - doesn't have the right to make such > > claims. > > > > You never asked kindly. I have always asked you kindly, probably more kindly than many others do (going through history, I just found many very kind questions I've been asking you). For the specific issue about the subtitles patchset, you
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Monday, April 11, 2022 10:52 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > On Mon, Apr 11, 2022 at 10:10 PM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Anton Khirnov > > > Sent: Monday, April 11, 2022 10:29 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > Quoting Soft Works (2022-04-08 17:27:10) > > > > > Furthermore, remember that this is just the first step. There > will > > > be > > > > > further patchsets converting the other components. I intend to > > > > > upstream > > > > > them gradually one after the other. Your suggestion would > require > > > me > > > > > to > > > > > instead write the whole thing at once, fighting rebase > conflicts > > > all > > > > > the > > > > > way, and then submit it as a giant utterly unreviewable > patchset. > > > > > > > > That's not what I meant, but anyway it's not worth discussing > when > > > > it's a minority opinion. > > > > > > > > Just a practical question instead for planning purposes: > > > > > > > > Which timeframe do you expect for the whole process? > > > > When do you plan to start > > > > > > If you mean "start pushing the patches", then I intend to do that > as > > > they are reviewed and approved. I hope to send the upstreamable > > > version > > > of this set this week, if nobody has strong objectsions then I > might > > > push it after vacation, i.e. late April/early May. > > > > > > > and for how long do you think it will take until all further > > > patchsets > > > > will be submitted/applied? > > > > > > This is very hard to estimate accurately. A pessimistic guess > assuming > > > I > > > get stuck on every stupid thing would be end of this year, but I > hope > > > for things to go much faster. > > > > Thanks for the reply. I'm asking because I need to decide about the > > way I'm going to proceed with the subtitle filtering patchset. > > > > I think I will have to keep and continue this in private during this > > procedure as I don't have the resources to regularly adapt and sync > > from my (5.0 based) working branch back to the master branch. > > > > > That is big waste of resource when not implementing thing properly. >From my point of view, somebody who has never given any detailed reviews, didn't state what exactly(!) he would consider to be "improper" and never made any suggestion how the implementation would need to be changed to become "proper" - doesn't have the right to make such claims. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Monday, April 11, 2022 10:29 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Quoting Soft Works (2022-04-08 17:27:10) > > > Furthermore, remember that this is just the first step. There will > be > > > further patchsets converting the other components. I intend to > > > upstream > > > them gradually one after the other. Your suggestion would require > me > > > to > > > instead write the whole thing at once, fighting rebase conflicts > all > > > the > > > way, and then submit it as a giant utterly unreviewable patchset. > > > > That's not what I meant, but anyway it's not worth discussing when > > it's a minority opinion. > > > > Just a practical question instead for planning purposes: > > > > Which timeframe do you expect for the whole process? > > When do you plan to start > > If you mean "start pushing the patches", then I intend to do that as > they are reviewed and approved. I hope to send the upstreamable > version > of this set this week, if nobody has strong objectsions then I might > push it after vacation, i.e. late April/early May. > > > and for how long do you think it will take until all further > patchsets > > will be submitted/applied? > > This is very hard to estimate accurately. A pessimistic guess assuming > I > get stuck on every stupid thing would be end of this year, but I hope > for things to go much faster. Thanks for the reply. I'm asking because I need to decide about the way I'm going to proceed with the subtitle filtering patchset. I think I will have to keep and continue this in private during this procedure as I don't have the resources to regularly adapt and sync from my (5.0 based) working branch back to the master branch. Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] warning about configuration mismatch between ff* binaries and libraries
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Monday, April 11, 2022 9:47 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] warning about configuration mismatch > between ff* binaries and libraries > > Quoting zhilizhao(赵志立) (2022-04-11 05:15:59) > > > > > > > On Apr 11, 2022, at 5:31 AM, Dominik 'Rathann' Mierzejewski > wrote: > > > > > > Dear Developers! > > > I'm curious about the warning about configuration mismatch between > ff* > > > binaries and the libraries introduced in: > > > > > > > https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/9120e2cd3fadfa60269e94 > f97fc8107974c586fc > > > > > > At Fedora, we're interested in having two builds: one built with > limited > > > set of codecs[1] that is legally distributable in the US (since > Red Hat, > > > Fedora sponsor is a US-based company) and another, with a more > complete > > > set of codecs[2] distributed by RPM Fusion. The builds are > intended to > > > be drop-in replacements. We'd like to be able to distribute just > one set > > > of ff* binaries and only have two different interchangeable builds > of > > > libraries. > > > > > > The idea is to have the builds differ by enabled codecs only and > have > > > the rest of the configuration the same (even if it's not the case > > > today). > > > > > > So, does that warning still make sense today? Will something break > > > if we swap the libraries but keep the binary on user systems? > > > > That’s exactly an example of why the warning is useful. Otherwise > user can > > be confused why some codecs are missing while banner says they are > enabled > > by configure. So yes, the warning still make sense. > > I would prefer to not show all that noise in the banner, which would > also resolve the confusion. Build information should be reduced to > loglevel verbose IMO. I agree and I would suggest reconsidering James' earlier patch for this. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Thursday, April 7, 2022 10:33 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Quoting Soft Works (2022-04-06 18:29:53) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Anton Khirnov > > > Sent: Wednesday, April 6, 2022 10:42 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > Quoting Soft Works (2022-04-05 23:05:57) > > > > do I understand it right that there won't be a single-thread > > > > operation mode that replicates/corresponds the current behavior? > > > > > > Correct, maintaining a single-threaded mode would require massive > > > amounts of extra effort for questionable gain. > > > > The gain is not to be seen in having an alternate run-mode in > > longer-term term perspective. It is about avoiding a single > > point-of-no-return change which may have fundamental consequences > > and impose debt on other developers when it would no longer be > possible > > to compare to the previous mode of operation. > > I don't understand where your apocalyptic language is coming from. > Yes, > overall this will be a very big change. Should it turn out that the switch will go smoothly and easily without (minimal) issues, then I'll sincerely admit that my concerns were exaggerated and unjustified. > > Threads are not magic. It is ubiquitous well-understood technology > that > has been around for decades. Yes, they do involve their own > challenges, > but so does everything else, including the status quo. > > The current architecture of ffmpeg.c is, in my opinion, unsustainable. Agreed. > There is barely any separation of responsibilities. E.g. filtering > code > will modify muxer state, etc. It is very hard to understand the code > behaviour in various scenarious (we have SO MANY options), and how > will > some code change affect it. This makes adding new major features much > more painful than it should be. I am firmly convinced that these > patches > will make the code significantly easier to understand. No doubt about that. > Sorry, this does not seem a reasonable request to me. For one thing, > the > only advantage you claim involve highly hypothetical doomsday > scenarios > where you cannot compare to the old code and so ALL IS LOST. That > smells > like a cargo cult. The current code is not holy scripture determining > how things should work for all eternity. It has bugs, inconsistencies, > and poorly handled corner cases (some of which are fixed by this very > patchset). As mentioned, the concern is about the transition not about carrying this for a long time. > Furthermore, remember that this is just the first step. There will be > further patchsets converting the other components. I intend to > upstream > them gradually one after the other. Your suggestion would require me > to > instead write the whole thing at once, fighting rebase conflicts all > the > way, and then submit it as a giant utterly unreviewable patchset. That's not what I meant, but anyway it's not worth discussing when it's a minority opinion. Just a practical question instead for planning purposes: Which timeframe do you expect for the whole process? When do you plan to start and for how long do you think it will take until all further patchsets will be submitted/applied? Thanks, sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Kieran Kunhya > Sent: Wednesday, April 6, 2022 7:57 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > > > > Not gonna happen, not gonna block progress because of whim of single > random > > contributor. > > > > Agreed, this patch series is as important as buffer referencing. We > can't > let holdouts block progress. RFC Patch => Opinionated Response != Blocking ___ 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] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Wednesday, April 6, 2022 10:42 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Quoting Soft Works (2022-04-05 23:05:57) > > do I understand it right that there won't be a single-thread > > operation mode that replicates/corresponds the current behavior? > > Correct, maintaining a single-threaded mode would require massive > amounts of extra effort for questionable gain. The gain is not to be seen in having an alternate run-mode in longer-term term perspective. It is about avoiding a single point-of-no-return change which may have fundamental consequences and impose debt on other developers when it would no longer be possible to compare to the previous mode of operation. > If I understand correctly what you're suggesting then I don't believe > this approach is feasible. The goal is not "add threading to improve > performance", keeping everything else intact as much as possible. The > goal is "improve architecture to make the code easier to > understand/maintain/extend", threads are a means towards that goal. > The > fact that this should also improve throughput is more of a nice side > effect than anything else. > > This patchset already changes behaviour in certain cases, making the > output more predictable and consistent. Reordering it somehow to > separate "semantically neutral" patches would require vast amounts of > extra work. Note that progressing at all without obviously breaking > anything is already quite hard --- I've been working on this since > November and this is just the first step. I really do not want to make > my work 10x harder for the vague benefit of maybe making some > debugging > slightly easier. I understand that, but I'm not talking about re-development. Let me try explain it in a different way: What I mean is to go through your patches one after another but apply only those parts that do not affect the current single-threaded execution flow - effectively separating out those parts. Then, you go through the remaining changes and make corresponding "similar" changes to the working code, making it get as close as possible to your original code. It's an iterative process. At the end you should have just a small set of changes left which make up the difference between the working code (still following the traditional flow) and the threaded execution flow. That last set of differences can be finally applied in a way that it can be activated/deactivated by an option. When you have been working on this for so long already, then this would make up just a small part of the total work. Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Wednesday, April 6, 2022 1:17 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > On Tue, Apr 5, 2022 at 11:20 PM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > Paul > > > B Mahol > > > Sent: Tuesday, April 5, 2022 11:19 PM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > On Tue, Apr 5, 2022 at 11:06 PM Soft Works > > > wrote: > > > > > > > > > > > > > > > > -Original Message- > > > > > From: ffmpeg-devel On Behalf > Of > > > > > Anton Khirnov > > > > > Sent: Tuesday, April 5, 2022 9:46 PM > > > > > To: FFmpeg development discussions and patches > > > > de...@ffmpeg.org> > > > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a > threaded > > > > > architecture > > > > > > > > > > Quoting Michael Niedermayer (2022-04-05 21:15:42) > > > > > > On Mon, Apr 04, 2022 at 01:29:48PM +0200, Anton Khirnov > wrote: > > > > > > > Hi, > > > > > > > this WIP patchset is the first major part of my ongoing > work > > > to > > > > > change > > > > > > > ffmpeg.c architecture such that every > > > > > > > - demuxer > > > > > > > - decoder > > > > > > > - filtergraph > > > > > > > - encoder > > > > > > > - muxer > > > > > > > lives in its own thread. The advantages of doing this, > beyond > > > > > increased > > > > > > > throughput, would be enforced separation between these > > > components, > > > > > > > making the code more local and easier to reason about. > > > > > > > > > > > > > > This set implements threading for muxers. My tentative > plan is > > > to > > > > > > > continue with encoders and then filters. The patches still > > > need > > > > > some > > > > > > > polishing, especially the last one. Two FATE tests do not > yet > > > > > pass, this > > > > > > > will be fixed in later iterations. > > > > > > > > > > > > > > Meanwhile, comments on the overall approach are especially > > > > > welcome. > > > > > > > > > > > > I agree that cleanup/modularization to make the code easier > to > > > > > > understand is a good idea! > > > > > > Didnt really look at the patchset yet. > > > > > > I assume these changes have no real disadvantage ? > > > > > > > > > > Playing the devil's advocate, I can think of the following: > > > > > 1) ffmpeg.c will hard-depend on threads > > > > > 2) execution flow will become non-deterministic > > > > > 3) overall resource usage will likely go up due to inter- > thread > > > > >synchronization and overhead related to new objects > > > > > 4) large-scale code changes always carry a higher risk of > > > regressions > > > > > > > > > > re 1): should not be a problem for any serious system > > > > > re 2): I spent a lot of effort to ensure the _output_ remains > > > > >deterministic (it actually becomes more predictable for > > > some > > > > >cases) > > > > > re 3): I expect the impact to be small and negligible, > > > respectively, > > > > > but > > > > >would have to be measured once the conversion is > complete > > > > > re 4): the only way to avoid this completely would be to stop > > > > >development > > > > > > > > > > Overall, I believe the advantages far outweigh the potential > > > > > negatives. > > > > > > > > Hi, > > > > > > > > do I understand it right that there won't be a single
Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Tuesday, April 5, 2022 11:19 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > On Tue, Apr 5, 2022 at 11:06 PM Soft Works > wrote: > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Anton Khirnov > > > Sent: Tuesday, April 5, 2022 9:46 PM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > > > architecture > > > > > > Quoting Michael Niedermayer (2022-04-05 21:15:42) > > > > On Mon, Apr 04, 2022 at 01:29:48PM +0200, Anton Khirnov wrote: > > > > > Hi, > > > > > this WIP patchset is the first major part of my ongoing work > to > > > change > > > > > ffmpeg.c architecture such that every > > > > > - demuxer > > > > > - decoder > > > > > - filtergraph > > > > > - encoder > > > > > - muxer > > > > > lives in its own thread. The advantages of doing this, beyond > > > increased > > > > > throughput, would be enforced separation between these > components, > > > > > making the code more local and easier to reason about. > > > > > > > > > > This set implements threading for muxers. My tentative plan is > to > > > > > continue with encoders and then filters. The patches still > need > > > some > > > > > polishing, especially the last one. Two FATE tests do not yet > > > pass, this > > > > > will be fixed in later iterations. > > > > > > > > > > Meanwhile, comments on the overall approach are especially > > > welcome. > > > > > > > > I agree that cleanup/modularization to make the code easier to > > > > understand is a good idea! > > > > Didnt really look at the patchset yet. > > > > I assume these changes have no real disadvantage ? > > > > > > Playing the devil's advocate, I can think of the following: > > > 1) ffmpeg.c will hard-depend on threads > > > 2) execution flow will become non-deterministic > > > 3) overall resource usage will likely go up due to inter-thread > > >synchronization and overhead related to new objects > > > 4) large-scale code changes always carry a higher risk of > regressions > > > > > > re 1): should not be a problem for any serious system > > > re 2): I spent a lot of effort to ensure the _output_ remains > > >deterministic (it actually becomes more predictable for > some > > >cases) > > > re 3): I expect the impact to be small and negligible, > respectively, > > > but > > >would have to be measured once the conversion is complete > > > re 4): the only way to avoid this completely would be to stop > > >development > > > > > > Overall, I believe the advantages far outweigh the potential > > > negatives. > > > > Hi, > > > > do I understand it right that there won't be a single-thread > > operation mode that replicates/corresponds the current behavior? > > > > Not that I wouldn't welcome the performance improvements, but one > > concern I have is debugging filtergraph operations. This is already > > a pretty tedious task in itself, because many relevant decisions > > are made in sub-sub-sub-sub-sub-functions, spread over many places. > > When adding an additional - not even deterministic - part to the > > game, it won't make things easier. It could even create situations > > where it could no longer be possible to replicate an error in a > > debugger - in case the existence of a debugger would cause a > variance > > within the constraints of the non-determinism range. > > > > > Can you elaborate more?, otherwise this is PEBKAC. You mean like WKOFAIT? ___ 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] [RFC] Switching ffmpeg.c to a threaded architecture
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Tuesday, April 5, 2022 9:46 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [RFC] Switching ffmpeg.c to a threaded > architecture > > Quoting Michael Niedermayer (2022-04-05 21:15:42) > > On Mon, Apr 04, 2022 at 01:29:48PM +0200, Anton Khirnov wrote: > > > Hi, > > > this WIP patchset is the first major part of my ongoing work to > change > > > ffmpeg.c architecture such that every > > > - demuxer > > > - decoder > > > - filtergraph > > > - encoder > > > - muxer > > > lives in its own thread. The advantages of doing this, beyond > increased > > > throughput, would be enforced separation between these components, > > > making the code more local and easier to reason about. > > > > > > This set implements threading for muxers. My tentative plan is to > > > continue with encoders and then filters. The patches still need > some > > > polishing, especially the last one. Two FATE tests do not yet > pass, this > > > will be fixed in later iterations. > > > > > > Meanwhile, comments on the overall approach are especially > welcome. > > > > I agree that cleanup/modularization to make the code easier to > > understand is a good idea! > > Didnt really look at the patchset yet. > > I assume these changes have no real disadvantage ? > > Playing the devil's advocate, I can think of the following: > 1) ffmpeg.c will hard-depend on threads > 2) execution flow will become non-deterministic > 3) overall resource usage will likely go up due to inter-thread >synchronization and overhead related to new objects > 4) large-scale code changes always carry a higher risk of regressions > > re 1): should not be a problem for any serious system > re 2): I spent a lot of effort to ensure the _output_ remains >deterministic (it actually becomes more predictable for some >cases) > re 3): I expect the impact to be small and negligible, respectively, > but >would have to be measured once the conversion is complete > re 4): the only way to avoid this completely would be to stop >development > > Overall, I believe the advantages far outweigh the potential > negatives. Hi, do I understand it right that there won't be a single-thread operation mode that replicates/corresponds the current behavior? Not that I wouldn't welcome the performance improvements, but one concern I have is debugging filtergraph operations. This is already a pretty tedious task in itself, because many relevant decisions are made in sub-sub-sub-sub-sub-functions, spread over many places. When adding an additional - not even deterministic - part to the game, it won't make things easier. It could even create situations where it could no longer be possible to replicate an error in a debugger - in case the existence of a debugger would cause a variance within the constraints of the non-determinism range. >From another point of view, this is a change, so fundamental like ffmpeg(.c) hasn't seen in a long time. I would at least suppose that this could cause issues at many ends, and from experience, there may be additional ends where it's rather unexpected to have effects. In that context, I think that doing a change of such a wide scope in an irreversible way like this, would impose quite a burden on many other developers, because sooner or later, other developers will run into situations where something is no longer working like before and you'll regularly wonder whether this might be a consequence of ffmpeg.c threading change or caused by other changes. But then, you won't be able anymore to bisect on that suspicion, because the threading change can't be reverted and (as long as it's not shortly after the change) there might have been too many other changes to easily port them back to a state before the threading change. I wonder whether this couldn't be done in a way that the current behavior can be preserved and activated by option? Wouldn't it be possible to follow an approach like this: - Assuming the code would be fine and it would mark the desired end result - Put it aside and start over from the current HEAD - Iteratively morph the code current code in a (possibly) long sequence of refactoring operations where every single one (and hence in sum) are semantically neutral - until the code is turned more and more into what has already been developed - eventually, only few differences will be left, and these can be made switchable by an option - as a result, both - old and new operation modes would be available. I don't know whether there's a name to this approach, probably there is, yet I never cared. Way more important is that I always had good results following this methodology. The funny thing about it is, that when you have a reliable tooling for refactoring, you can even stop thinking (well, sort of..) while transforming the code. Also, when you can't
Re: [FFmpeg-devel] [PATCH] avfilter/f_realtime: add support for commands
> -Original Message- > From: ffmpeg-devel On Behalf Of Paul > B Mahol > Sent: Friday, March 25, 2022 12:19 PM > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH] avfilter/f_realtime: add support for > commands > > Signed-off-by: Paul B Mahol > --- > doc/filters.texi | 4 > libavfilter/f_realtime.c | 4 +++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index d70ac3e237..1d56d24819 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -26757,6 +26757,10 @@ A processing speed faster than what is > possible without these filters cannot > be achieved. Didn't you rather mean to say: A processing speed faster than what is possible with these filters cannot.. Best wishes, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage size to libass
> -Original Message- > From: ffmpeg-devel On Behalf Of > Oneric > Sent: Tuesday, March 22, 2022 5:28 PM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass > storage size to libass > > On Mon, Mar 14, 2022 at 20:06:39 +0100, Oneric wrote: > > Due to a quirk of the ASS format some tags depend on the exact > storage > > resolution of the video, so tell libass via ass_set_storage_size. > > [...] > > On Mon, Mar 14, 2022 at 20:21:47 +, Soft Works wrote: > > [...] > > > > Ah, alright, the blur setting goes deeper. Thanks for the > explanation. > > > > LGTM, then! > > > ping It's not on me. I would have merged it, but I don't have push permissions. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 3/3] libavcodec/qsvdec: using suggested num to set init_pool_size
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Wenbin Chen
> Sent: Friday, March 18, 2022 7:25 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2 3/3] libavcodec/qsvdec: using
> suggested num to set init_pool_size
>
> The init_pool_size is set to be 64 and it is too many.
> Use IOSurfQuery to get NumFrameSuggest which is the suggested
> number of frame that needed to be allocated when initializing the
> decoder.
> Considering that the hevc_qsv encoder uses the most frame buffer,
> async is 4 (default) and max_b_frames is 8 (default) and decoder
> may followed by VPP, use NumFrameSuggest + 16 to set init_pool_size.
>
> Signed-off-by: Wenbin Chen
> Signed-off-by: Guangxin Xu
> ---
> libavcodec/qsvdec.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
> index 210bd0c1d5..9875d3d632 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -88,7 +88,7 @@ typedef struct QSVContext {
> uint32_t fourcc;
> mfxFrameInfo frame_info;
> AVBufferPool *pool;
> -
> +int suggest_pool_size;
> int initialized;
>
> // options set by the caller
> @@ -275,7 +275,7 @@ static int qsv_decode_preinit(AVCodecContext
> *avctx, QSVContext *q, enum AVPixel
> hwframes_ctx->height= FFALIGN(avctx-
> >coded_height, 32);
> hwframes_ctx->format= AV_PIX_FMT_QSV;
> hwframes_ctx->sw_format = avctx->sw_pix_fmt;
> -hwframes_ctx->initial_pool_size = 64 + avctx-
> >extra_hw_frames;
> +hwframes_ctx->initial_pool_size = q->suggest_pool_size + 16 +
> avctx->extra_hw_frames;
> frames_hwctx->frame_type=
> MFX_MEMTYPE_VIDEO_MEMORY_DECODER_TARGET;
>
> ret = av_hwframe_ctx_init(avctx->hw_frames_ctx);
> @@ -793,6 +793,9 @@ static int qsv_process_data(AVCodecContext *avctx,
> QSVContext *q,
> }
>
> if (q->reinit_flag || !q->session || !q->initialized) {
> +mfxFrameAllocRequest request;
> +memset(, 0, sizeof(request));
> +
> q->reinit_flag = 0;
> ret = qsv_decode_header(avctx, q, pkt, pix_fmt, );
> if (ret < 0) {
> @@ -802,12 +805,19 @@ static int qsv_process_data(AVCodecContext
> *avctx, QSVContext *q,
> av_log(avctx, AV_LOG_ERROR, "Error decoding
> header\n");
> goto reinit_fail;
> }
> +param.IOPattern = q->iopattern;
>
> q->orig_pix_fmt = avctx->pix_fmt = pix_fmt =
> ff_qsv_map_fourcc(param.mfx.FrameInfo.FourCC);
>
> avctx->coded_width = param.mfx.FrameInfo.Width;
> avctx->coded_height = param.mfx.FrameInfo.Height;
>
> +ret = MFXVideoDECODE_QueryIOSurf(q->session, ,
> );
> +if (ret < 0)
> +return ff_qsv_print_error(avctx, ret, "Error querying IO
> surface");
> +
> +q->suggest_pool_size = request.NumFrameSuggested;
> +
> ret = qsv_decode_preinit(avctx, q, pix_fmt, );
> if (ret < 0)
> goto reinit_fail;
> --
Thanks for the patch! I have that on my list for quite a while.
Will look at it shortly.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage size to libass
> -Original Message- > From: ffmpeg-devel On Behalf Of Oneric > Sent: Monday, March 14, 2022 9:08 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage > size to libass > > On Mon, Mar 14, 2022 at 19:57:05 +, Soft Works wrote: > > > > ass_set_pixel_aspect() is setting settings.par and if I'm not > mistaken, > > > > an existing par setting leads to the storage size setting to be > ignored: > > > > > > It’s not overridden. Only the explicit PAR is currently preferd over > the > > > implicit derivation from storage and frame size. However as I stated > in > > > the patch description and the comment: > > > “some tags depend on the exact storage resolution of the video” > > > “it actually goes farther than just the aspect ratio” > > > > > > > I found only one other place where storage_h is used (for determining > > blur size) but I didn't find any other usage in the libass source code. > > That's what I'm wondering about. > > Well, blur is one of the things that depend on it. If you follow the usage > of the blur scale, you'll see it also plays a role in the projection > matrix for 3D-transforms (what the provided samples use) and if > ScaledBorderAndShadow is not set to "yes", it also affects some other > scaling values. > > This unfortunate dependence is a result of how SSA and then ASS > histoically developed and required to maintain compaitibility with > existing subtitles. Ah, alright, the blur setting goes deeper. Thanks for the explanation. LGTM, then! ___ 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] avfilter/vf_subtitles: pass storage size to libass
> -Original Message- > From: ffmpeg-devel On Behalf Of Oneric > Sent: Monday, March 14, 2022 8:50 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage > size to libass > > On Mon, Mar 14, 2022 at 19:35:36 +, Soft Works wrote: > > > > I've been at the same point some time ago where I wondered why ffmpeg is > > not setting this, but then I had found that it is overridden by the call > > to ass_set_pixel_aspect(). > > > > ass_set_pixel_aspect() is setting settings.par and if I'm not mistaken, > > an existing par setting leads to the storage size setting to be ignored: > > It’s not overridden. Only the explicit PAR is currently preferd over the > implicit derivation from storage and frame size. However as I stated in > the patch description and the comment: > “some tags depend on the exact storage resolution of the video” > “it actually goes farther than just the aspect ratio” > I found only one other place where storage_h is used (for determining blur size) but I didn't find any other usage in the libass source code. That's what I'm wondering about. Thanks, sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage size to libass
> -Original Message-
> From: ffmpeg-devel On Behalf Of Oneric
> Sent: Monday, March 14, 2022 8:07 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Oneric
> Subject: [FFmpeg-devel] [PATCH] avfilter/vf_subtitles: pass storage size
> to libass
>
> Due to a quirk of the ASS format some tags depend on the exact storage
> resolution of the video, so tell libass via ass_set_storage_size.
>
> ---
> ass_set_storage_size exists since libass 0.10.2;
> ffmpeg since 5.0 already requires 0.11.0.
>
> This resolution dependences of ASS was already recognised when the
> original_size parameter was added, but it actually goes farther than
> just the aspect ratio. Conveniently this parameter still has all the
> required information to retain rendering after resizing :)
>
> Sample files to show the difference can be found eg here
> https://code.videolan.org/videolan/vlc/uploads/b54e0761d0d3f4f79b2947ffb83
> a3b59/vlc-issue_libass-storage-size.tar.xz
>
> ./ffmpeg -i test_1080p.mkv -filter:v ass=./test_1080p.ass tmp_1080.mkv
> ./ffmpeg -i anamorphic_s720x576_d1024x576.mkv -filter:v
> ass=./anamorphic_s720x576_d1024x576.ass tmp_anam.mkv
>
> ---
> libavfilter/vf_subtitles.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c
> index 3fc4eeb63d..af6352b315 100644
> --- a/libavfilter/vf_subtitles.c
> +++ b/libavfilter/vf_subtitles.c
> @@ -146,9 +146,14 @@ static int config_input(AVFilterLink *inlink)
> ff_draw_init(>draw, inlink->format, ass->alpha ?
> FF_DRAW_PROCESS_ALPHA : 0);
>
> ass_set_frame_size (ass->renderer, inlink->w, inlink->h);
> -if (ass->original_w && ass->original_h)
> +if (ass->original_w && ass->original_h) {
> ass_set_pixel_aspect(ass->renderer, (double)inlink->w / inlink->h
> /
> ((double)ass->original_w / ass-
> >original_h));
> +ass_set_storage_size(ass->renderer, ass->original_w, ass-
> >original_h);
> +} else {
> +ass_set_storage_size(ass->renderer, inlink->w, inlink->h);
> +}
> +
> if (ass->shaping != -1)
> ass_set_shaper(ass->renderer, ass->shaping);
>
Hi,
thanks for the patch!
I've been at the same point some time ago where I wondered why ffmpeg is
not setting this, but then I had found that it is overridden by the call
to ass_set_pixel_aspect().
ass_set_pixel_aspect() is setting settings.par and if I'm not mistaken,
an existing par setting leads to the storage size setting to be ignored:
https://github.com/libass/libass/blob/5f0e8450f834894b2745238e3d32ff4878710ec8/libass/ass_render.c#L2891-L2903
But perhaps I'm missing something..
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC hwaccel code
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Wednesday, February 9, 2022 6:49 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC > hwaccel code > > Quoting Soft Works (2022-02-08 22:34:42) > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Anton Khirnov > > > Sent: Tuesday, February 8, 2022 10:37 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC > > > hwaccel code > > > > > > Quoting Soft Works (2022-02-07 03:18:54) > > > > I sometimes wonder whether there exists a single API user who > > > > really understands this (very special) kind of logic and > > > > would make decisions based on that understanding. > > > > > > > > When it's not even fully understood internally, how should it > > > > be understood externally? > > > > > > The rule for API users is simple: you are not allowed to assume a > > > specific component (like a decoder, demuxer or hwaccel) will be > > > available at runtime*. You are supposed to check for it using the > APIs > > > provided for this purpose. In this case, AV_PIX_FMT_XVMC will just > > > stop > > > being offered in get_format(). > > > > > > Not to mention that I very much doubt there are any users of xvmc > > > left, > > > besides the original mplayer. > > > > > > * unless you are running with a very specific verified build, in > which > > > case a removal like this should be caught at the build stage > > > > Thanks for the explanation. I misunderstood this a bit - when the > > only effect of the "removal" is an output like when the libs > wouldn't > > have been compiled with that part, then that's maybe nothing to > > be blamed for. > > On the other side: when somebody is using it and updates the libs > > to a point where it's missing, it will be broken for her/him. > > So the outcome might not be _that_ different for any other breaking > > changes being made. > > I agree that in general removing random components would be a bad > thing > to do, even though it's not technically an API break. That's why we > don't do it if there's any chance the component in question is useful. > > The argument _in this specific case_ is that xvmc is not useful to > anyone and has not been for a very long time, so no valid use cases > are > broken by its removal. In fact its only user ever I'm aware of is the > original mplayer (mplayer2, which was later forked into mpv, dropped > it > in 2011). Sure. Don't understand my reply as an objection I don't even know what xvmc is (or was). I rather see the burden and effort that it takes to retain all those compatibility paths and at the same time how it is blocking innovation and progress. Compatibility is important - without question, but doing it in a way that libs from different versions can be combined, is a somewhat crazy endeavor. I keep wondering who would be the developer whose dreams this might fulfil.. The discussion about that seems to have gotten stuck about whether to merge libs together or not, or how, but I haven't followed in detailed, so please excuse the question (which has probably been covered before): Why can't ffmpeg simply declare that starting from version X, it will be a requirement that all libs are from the same version? (of course after equalizing) BR, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC hwaccel code
> -Original Message- > From: ffmpeg-devel On Behalf Of > Anton Khirnov > Sent: Tuesday, February 8, 2022 10:37 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC > hwaccel code > > Quoting Soft Works (2022-02-07 03:18:54) > > I sometimes wonder whether there exists a single API user who > > really understands this (very special) kind of logic and > > would make decisions based on that understanding. > > > > When it's not even fully understood internally, how should it > > be understood externally? > > The rule for API users is simple: you are not allowed to assume a > specific component (like a decoder, demuxer or hwaccel) will be > available at runtime*. You are supposed to check for it using the APIs > provided for this purpose. In this case, AV_PIX_FMT_XVMC will just > stop > being offered in get_format(). > > Not to mention that I very much doubt there are any users of xvmc > left, > besides the original mplayer. > > * unless you are running with a very specific verified build, in which > case a removal like this should be caught at the build stage Thanks for the explanation. I misunderstood this a bit - when the only effect of the "removal" is an output like when the libs wouldn't have been compiled with that part, then that's maybe nothing to be blamed for. On the other side: when somebody is using it and updates the libs to a point where it's missing, it will be broken for her/him. So the outcome might not be _that_ different for any other breaking changes being made. And that's what I meant to point out: On one side, we are carefully curating those versioning changes (individually for each lib!) with a complex set of rules and considerations to be made, while in the majority of cases I tend to believe that not many really care that much about it, instead sitting there, happily using it until the moment "Oh it's broken - - need to do something - fix it and go on.." :-) softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC hwaccel code
> -Original Message- > From: ffmpeg-devel On Behalf Of > James Almer > Sent: Monday, February 7, 2022 2:53 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH 1/4] lavc/mpeg*: drop the XvMC > hwaccel code > > On 2/6/2022 10:46 PM, Andreas Rheinhardt wrote: > > Anton Khirnov: > >> XvMC was last relevant over 10 years ago, if ever. There is no > reason to > >> use it today. > >> --- > >> MAINTAINERS| 1 - > >> configure | 9 - > >> libavcodec/Makefile| 2 - > >> libavcodec/avcodec.h | 12 -- > >> libavcodec/error_resilience.c | 9 - > >> libavcodec/hwaccels.h | 2 - > >> libavcodec/hwconfig.h | 2 - > >> libavcodec/mpeg12dec.c | 43 > >> libavcodec/mpegvideo.c | 6 - > >> libavcodec/mpegvideo.h | 2 - > >> libavcodec/mpegvideo_xvmc.c| 376 > - > >> libavcodec/x86/blockdsp_init.c | 4 - > >> libavcodec/xvmc_internal.h | 31 --- > >> 13 files changed, 499 deletions(-) > >> delete mode 100644 libavcodec/mpegvideo_xvmc.c > >> delete mode 100644 libavcodec/xvmc_internal.h > >> > >> diff --git a/MAINTAINERS b/MAINTAINERS > >> index c065e94498..f33ccbd1d9 100644 > >> --- a/MAINTAINERS > >> +++ b/MAINTAINERS > >> @@ -266,7 +266,6 @@ Codecs: > >> xan.c Mike Melanson > >> xbm* Paul B Mahol > >> xface Stefano Sabatini > >> - xvmc.cIvan Kalvachev > >> xwd* Paul B Mahol > >> > >> Hardware acceleration: > >> diff --git a/configure b/configure > >> index 5b19a35f59..6dbe108284 100755 > >> --- a/configure > >> +++ b/configure > >> @@ -1900,7 +1900,6 @@ HWACCEL_AUTODETECT_LIBRARY_LIST=" > >> videotoolbox > >> vulkan > >> v4l2_m2m > >> -xvmc > >> " > >> > >> # catchall list of things that require external libs to link > >> @@ -3006,7 +3005,6 @@ vaapi_x11_deps="xlib_x11" > >> videotoolbox_hwaccel_deps="videotoolbox pthreads" > >> videotoolbox_hwaccel_extralibs="-framework QuartzCore" > >> vulkan_deps_any="libdl LoadLibrary" > >> -xvmc_deps="X11_extensions_XvMClib_h" > >> > >> av1_d3d11va_hwaccel_deps="d3d11va DXVA_PicParams_AV1" > >> av1_d3d11va_hwaccel_select="av1_decoder" > >> @@ -3054,16 +3052,12 @@ mjpeg_nvdec_hwaccel_deps="nvdec" > >> mjpeg_nvdec_hwaccel_select="mjpeg_decoder" > >> mjpeg_vaapi_hwaccel_deps="vaapi" > >> mjpeg_vaapi_hwaccel_select="mjpeg_decoder" > >> -mpeg_xvmc_hwaccel_deps="xvmc" > >> -mpeg_xvmc_hwaccel_select="mpeg2video_decoder" > >> mpeg1_nvdec_hwaccel_deps="nvdec" > >> mpeg1_nvdec_hwaccel_select="mpeg1video_decoder" > >> mpeg1_vdpau_hwaccel_deps="vdpau" > >> mpeg1_vdpau_hwaccel_select="mpeg1video_decoder" > >> mpeg1_videotoolbox_hwaccel_deps="videotoolbox" > >> mpeg1_videotoolbox_hwaccel_select="mpeg1video_decoder" > >> -mpeg1_xvmc_hwaccel_deps="xvmc" > >> -mpeg1_xvmc_hwaccel_select="mpeg1video_decoder" > >> mpeg2_d3d11va_hwaccel_deps="d3d11va" > >> mpeg2_d3d11va_hwaccel_select="mpeg2video_decoder" > >> mpeg2_d3d11va2_hwaccel_deps="d3d11va" > >> @@ -3078,8 +3072,6 @@ mpeg2_vdpau_hwaccel_deps="vdpau" > >> mpeg2_vdpau_hwaccel_select="mpeg2video_decoder" > >> mpeg2_videotoolbox_hwaccel_deps="videotoolbox" > >> mpeg2_videotoolbox_hwaccel_select="mpeg2video_decoder" > >> -mpeg2_xvmc_hwaccel_deps="xvmc" > >> -mpeg2_xvmc_hwaccel_select="mpeg2video_decoder" > >> mpeg4_nvdec_hwaccel_deps="nvdec" > >> mpeg4_nvdec_hwaccel_select="mpeg4_decoder" > >> mpeg4_vaapi_hwaccel_deps="vaapi" > >> @@ -6320,7 +6312,6 @@ check_headers unistd.h > >> check_headers valgrind/valgrind.h > >> check_func_headers VideoToolbox/VTCompressionSession.h > VTCompressionSessionPrepareToEncodeFrames -framework VideoToolbox > >> check_headers windows.h > >> -check_headers X11/extensions/XvMClib.h > >> check_headers asm/types.h > >> > >> # it seems there are versions of clang in some distros that try > to use the > >> diff --git a/libavcodec/Makefile b/libavcodec/Makefile > >> index cfc70a3eaf..6076b4ad80 100644 > >> --- a/libavcodec/Makefile > >> +++ b/libavcodec/Makefile > >> @@ -963,7 +963,6 @@ OBJS-$(CONFIG_MJPEG_VAAPI_HWACCEL)+= > vaapi_mjpeg.o > >> OBJS-$(CONFIG_MPEG1_NVDEC_HWACCEL)+= nvdec_mpeg12.o > >> OBJS-$(CONFIG_MPEG1_VDPAU_HWACCEL)+= vdpau_mpeg12.o > >> OBJS-$(CONFIG_MPEG1_VIDEOTOOLBOX_HWACCEL) += videotoolbox.o > >> -OBJS-$(CONFIG_MPEG1_XVMC_HWACCEL) += mpegvideo_xvmc.o > >> OBJS-$(CONFIG_MPEG2_D3D11VA_HWACCEL) += dxva2_mpeg2.o > >> OBJS-$(CONFIG_MPEG2_DXVA2_HWACCEL)+= dxva2_mpeg2.o > >> OBJS-$(CONFIG_MPEG2_NVDEC_HWACCEL)+= nvdec_mpeg12.o > >> @@ -971,7 +970,6 @@ OBJS-$(CONFIG_MPEG2_QSV_HWACCEL) += > qsvdec.o > >> OBJS-$(CONFIG_MPEG2_VAAPI_HWACCEL)+= vaapi_mpeg2.o > >>
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft
> Works
> Sent: Sunday, February 6, 2022 2:09 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> > Oneric
> > Sent: Saturday, February 5, 2022 11:00 PM
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> > On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> > > Let's try to approach this from a different side. Which case is
> > > your [1/2] commit actually supposed to fix?
> >
> > Escape backslashes when converting from WebVTT to not accidentally
> > introduce active ASS sequences and replace the wrong backslash-
> escape
> > in ff_ass_bprint_text_event with an escape that actually works.
> >
> > > How did you test your patch?
> > > Can we please go over an example?
> >
> > Take a look at the attached WebVTT file.
>
> Thanks a lot for the test file!
>
>
> > We expect the second event to be rendered like this,
> > as from WebVTT’s point of view it’s just normal text:
> >
> > our final \h approach \N into \ Coruscant.
> >
> > What we currently get after conversion to ASS is like this though
> > (pay attention to the number of spaces):
> >
> > our final approach
> > into \ Coruscant.
>
> Yup, no doubt that this is wrong.
>
>
> > If instead the word-joiner is appended as in my patch, the
> > visual output matches the expectation (mail does not contain
> U+2060):
> >
> > our final \h approach \N into \ Coruscant.
>
> I can also confirm that your patch would "work" with regards
> to ass output when trying with both "old" libass and new libass.
> I haven't tried with other implementations yet, but when this
> would turn out to be working with all usual implementations,
> I might even be OK with the word-joiner.
>
> But this is where the agreement ends.
>
> - If at all, the word-joiner insertion would need to be
> limited to ASS output ONLY
> - it would need to be controllable through an option in the ASS
> encoder
> - The word joiners should not be used in internal processing and
> only be (optionally) added when encoding to ass
> - Unfortunately, the FATE tests for the other subtitle formats
> do not include these sequences in the test source files, and
> that means, before making such change that might potentially
> affect all other text subtitle encoders, those sequences would
> need to be added to make sure these conversion won't be affected
> - Generally, those changes (also the BIDI mark insertion) should
> be evaluated with regards to all text subtitle encoders,
> making sure there's no side effect.
>
> You said:
>
> > I’m not interested in reworking ffmpeg’s internal subtitle handling.
> > The proposed patch is a clear improvement over the status quo which
> > is plain incorrect. Within reasonable effort and sound arguments for
> > it adjustments to the patch can be made; reworking ffmpeg internals
> is
> > imo not “reasonable” effort to correct an uncontestedly wrong
> escape.
>
> And that is a problem. The changes you are proposing are making
> changes to ffmpeg’s internal subtitle handling, so you need to decide
> whether you want to work on it or not.
>
>
> > You have two options:
> [..]
> > Or go ahead and create your own patch.
>
> I will do this, but "only" on top of my subtitle filtering patchset
> because that's my current focus area and just two weeks ago I had to
> add a temporary hack for a related case which is about ASS dialog
> lines
> like:
>
> Dialogue: 0,0:00:00.00,0:00:05.00,Default,,0,0,0,,{comment text:
> \}
>
> Currently, ffmpeg does not recognize this as override code, even
> though
> it's valid in ASS that a backslash with the actual code doesn't appear
> immediately after the opening curly brace.
> What comes on top of this is that other subtitle decoders do NOT
> escape
> the curly braces like WebVTTdec and ass_bprint_text_event().
> When you look at SubRip_capability_tester.srt from the FATE suite, you
> can see that it contains ASS codes that are expected to be recognized
> and applied, but when there's normal text in curly braces without
> a backslash, it should be treated at normal text.
>
> This is qui
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Saturday, February 5, 2022 11:00 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> > Let's try to approach this from a different side. Which case is
> > your [1/2] commit actually supposed to fix?
>
> Escape backslashes when converting from WebVTT to not accidentally
> introduce active ASS sequences and replace the wrong backslash-escape
> in ff_ass_bprint_text_event with an escape that actually works.
>
> > How did you test your patch?
> > Can we please go over an example?
>
> Take a look at the attached WebVTT file.
Thanks a lot for the test file!
> We expect the second event to be rendered like this,
> as from WebVTT’s point of view it’s just normal text:
>
> our final \h approach \N into \ Coruscant.
>
> What we currently get after conversion to ASS is like this though
> (pay attention to the number of spaces):
>
> our final approach
> into \ Coruscant.
Yup, no doubt that this is wrong.
> If instead the word-joiner is appended as in my patch, the
> visual output matches the expectation (mail does not contain U+2060):
>
> our final \h approach \N into \ Coruscant.
I can also confirm that your patch would "work" with regards
to ass output when trying with both "old" libass and new libass.
I haven't tried with other implementations yet, but when this
would turn out to be working with all usual implementations,
I might even be OK with the word-joiner.
But this is where the agreement ends.
- If at all, the word-joiner insertion would need to be
limited to ASS output ONLY
- it would need to be controllable through an option in the ASS
encoder
- The word joiners should not be used in internal processing and
only be (optionally) added when encoding to ass
- Unfortunately, the FATE tests for the other subtitle formats
do not include these sequences in the test source files, and
that means, before making such change that might potentially
affect all other text subtitle encoders, those sequences would
need to be added to make sure these conversion won't be affected
- Generally, those changes (also the BIDI mark insertion) should
be evaluated with regards to all text subtitle encoders,
making sure there's no side effect.
You said:
> I’m not interested in reworking ffmpeg’s internal subtitle handling.
> The proposed patch is a clear improvement over the status quo which
> is plain incorrect. Within reasonable effort and sound arguments for
> it adjustments to the patch can be made; reworking ffmpeg internals is
> imo not “reasonable” effort to correct an uncontestedly wrong escape.
And that is a problem. The changes you are proposing are making
changes to ffmpeg’s internal subtitle handling, so you need to decide
whether you want to work on it or not.
> You have two options:
[..]
> Or go ahead and create your own patch.
I will do this, but "only" on top of my subtitle filtering patchset
because that's my current focus area and just two weeks ago I had to
add a temporary hack for a related case which is about ASS dialog lines
like:
Dialogue: 0,0:00:00.00,0:00:05.00,Default,,0,0,0,,{comment text: \}
Currently, ffmpeg does not recognize this as override code, even though
it's valid in ASS that a backslash with the actual code doesn't appear
immediately after the opening curly brace.
What comes on top of this is that other subtitle decoders do NOT escape
the curly braces like WebVTTdec and ass_bprint_text_event().
When you look at SubRip_capability_tester.srt from the FATE suite, you
can see that it contains ASS codes that are expected to be recognized
and applied, but when there's normal text in curly braces without
a backslash, it should be treated at normal text.
This is quite a mess that needs to be cleaned up with a plan and it's
all related. Like I said already: A central point to this is the escaping
and what's needed is a solution that can cover all of those things.
I had put this subject aside as I've been unsure about how to do it,
but this discussion has been very helpful to see the issues more clearly.
How about separating the BIDI part from your patch? I'd see only two
things remaining:
- Go through all text subtitle encoders and think/research whether
those marks would be acceptable in those formats or whether they
would need to be removed (like now)
- Think about whether the Unicode bidi marks should be replaced back
to the html-style codes
It's not these wouldn't work, but it's again about visibility and
I tend to think that it would be preferable to have them visible
in the out
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Saturday, February 5, 2022 2:20 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 23:24:58 +, Soft Works wrote:
> > You want to "pollute" gazillions of subtitle streams in the
> > world from multiple subtitle formats with invisible
> > characters in order to solve an escaping problem in ffmpeg?
>
> I do not consider using characters that are explicitly recommended to
> be
> used by Unicode to be “polluting”. Further consider that as mentioned
> invisible characters in ASS are not uncommon anyway already and
> conversion
> from ASS to something else are rare due to being generally lossy.
> Lossy
> with regards to typesetting that is, removing breaking hints in form
> of
> plain Unicode characters would be a new form of lossyness.
>
> > [From the other mail:]
> > I'm not into changing ffmpeg's ass output, it's all
> > about the internally used ass format and the escaping is
> > a central problem there.
>
> I’m not interested in reworking ffmpeg’s internal subtitle handling.
> The proposed patch is a clear improvement over the status quo which
> is plain incorrect. Within reasonable effort and sound arguments for
> it adjustments to the patch can be made; reworking ffmpeg internals is
> imo not “reasonable” effort to correct an uncontestedly wrong escape.
>
> You have two options:
> Either finally tell me what I asked about:
> where (as in which file and function) removing wordjoiners should
> even happen and where possible lingering “\\ → \” conversions
> presumably
> are and if it’s simple enough I can add a removal accompanied by a
> comment
> pointing out that this can go wrong.
> Or go ahead and create your own patch.
>
> ~~
>
> > > > I'm not sure whether all ffmpeg text-sub encoders can handle
> > > > those chars - which could be verified of course.
> > >
> > > Since it's in the BMP and ffmpeg already seems happy to assume
> some
> > > UTF-8
> > > support by converting everything to it, I'm not worried about this
> > > until
> > > proven wrong.
> >
> > Proven wrong: https://github.com/libass/libass/issues/507
>
> This issue is not at all wordjoiner specific despite the name.
> As far as I recall this never lead to wrong rendering.
> With HarfBuzz, the only fully featured shaping backend of libass,
> control characters were and are handled by HarfBuzz.
> And even with FriBiDi U+2060 was ignored since long before (2012)
> the linked issue was opened.
>
> What that issue really is about is a combination of two more general
> issues. libass is currently not caching failure to lookup a glyph
> leading
> to multiple messages and at worst a perf degradation if no font on the
> font pool contained a glyph for a particular glyph. And the
> realisation
> that libass’ font-fallback strategy is not ideal for prefix-type
> control
> characters, characters which visibly affect both neighbours and a few
> others.
> The word-joiner is only highlighted here as due to its usage as an
> backslash escape its commonly passed to libass and a high enough
> percentage of fonts doesn’t contain it to create reports about it.
>
>
> For further reference: U+2060 was added in Unicode 3.2 released 2002.
> If you want to strip it because it might not render correctly you
> should
> also strip most emoji, the uppercase eszett ẞ and several actively
> used writing systems in their entirety.
Let's try to approach this from a different side. Which case is
your [1/2] commit actually supposed to fix?
How did you test your patch?
Can we please go over an example?
Thanks,
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Friday, February 4, 2022 10:52 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 02:30:37 +0100, Andreas Rheinhardt wrote:
> > All text-based subtitles are supposed to be UTF-8 when they reach
> the
> > decoder; if it isn't, the user has to set the appropriate -
> sub_charenc
> > and -sub_charenc_mode.
> >
> > - Andreas
>
> Thanks for the info! Then at least the UTF-8 assumption
> is no problem after all.
>
>
[..]
> >
> > I'm not sure whether all ffmpeg text-sub encoders can handle
> > those chars - which could be verified of course.
>
> Since it's in the BMP and ffmpeg already seems happy to assume some
> UTF-8
> support by converting everything to it, I'm not worried about this
> until
> proven wrong.
Proven wrong: https://github.com/libass/libass/issues/507
> > Finally, those chars are a pest. I'm using them myself for a
> > specific use case, but when you don't know they are there, it can
> > drive you totally mad, eventually even thinking your system or
> > software is faulty.
> >
> > Example:
> >
> > Open your patch file [2/2] and search for the string
> > "123456\NAscending". You can see the string in two lines, but search
> > will only find one of them.
> >
> > Or just look at the two lines directly. They are preceded by + and -
> > even though both appear identical.
>
> Actually, I see this with helpful colouring lost here:
>
> -Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending:
> 123456\NAscending: 123456^M
> +Dialogue: 0,0:00:55.00,0:01:00.00,Default,,0,0,0,,Descending:
> <200f>123456<200e>\NAscending: 123456^M
I didn't say you won't be a able to find a viewer that can display
them. :-)
> More plain-text oriented editors likely won't show them though, yes.
Yes => pest
> > That might be true, but I think it's valid to say that such
> characters
> > are very unusual "original" subtitle sources and that's why I don't
> > think it's a good idea for ffmpeg to start injecting them.
>
> Don't underestimate what subtitle authors can come up with :)
Sure. But a subtitle author is responsible for their authored
subtitles while ffmpeg is responsible for encoding of large
part of the world's subtitles.
And from that same perspective I find the relation of this
proposal somewhat insane:
You want to "pollute" gazillions of subtitle streams in the
world from multiple subtitle formats with invisible
characters in order to solve an escaping problem in ffmpeg?
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Friday, February 4, 2022 10:19 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Fri, Feb 04, 2022 at 06:48:40 +, Soft Works wrote:
> > > [two-part message removed for brevity]
> >
> > I've found out where the \{ and \} escaping has come from: libass
>
> As already written in the commit-message of the first patch..
>
>
> You already noticed your proposal only works with VSFilters,
> but even without this it's a terrible approach. Consider:
> - fullwidth characters have different metrics then the "regular" ones
> - fullwidth and small characters have a different visual appearance
> - support for fullwidth and small characters in fonts is much rarer
>than support for plain {}
> - fullwidth characters are commonly used _as fullwidth characters_
>e.g. in text using one of the CJK writing systems.
>Replacing them with non-fullwidth variants when transforming
>away from ASS is guaranteed to be disastrous.
> - Not sure if applies, but something to keep in mind:
>{\r} is not a noop if the source-format had any sort of per-event
>styling which got prepended to the ASS event text before
>using the plain-text conversion for the rest of the event.
No, this doesn't apply from what I've seen, but {} might
still be preferable.
> As noted in the discussion of the libass issue you linked,
> it’s not unusual for ASS subtitle authors to employ
> fullwidth curly braces for displaying curly braces
> in all ASS-renderers. However, they have tight control over the
> fonts used and can carefully select them to match the visual
> appearance and compensate differing metrics with bespoke
> local adjustments to \fs and negative \fsp.
> ffmpeg does not have tight control over the fonts and it'd be silly
> to require users to pass in special fonts just to render curly braces.
I basically agree to everything you say - except that there's a
little misunderstanding. Maybe I haven't explained well enough.
We use ASS as the internal format to which all text subtitles are
decoded and from which all text subtitles are encoded for output
and for the upcoming subtitle filtering it's also the one and only
format for text subtitles.
BUT: That does not necessarily mean that the internally used
ASS must be exactly the same that we're outputting when encoding
to ASS. And that's why we need to consider this as two separate
steps. It would also be possible to have options at the ass encoder
to control the compatibility of the encoded ASS output.
That internal ASS format already has some quirks that some had
introduced in order to achieve certain things when other subtitle
formats are involved at the input and at the output. That's why
we should not continue adding one workaround on top of another
but try to clean those things up instead.
With your submission, you are actually pointing at a core point
of evil: the escaping of braces in combination with the backslash
logic introduces an unresolvable ambiguity. And when we don't
clean that up, it won't be possible to get on a sane path.
> If you want to make the rendering in VSFilters not complettly broken,
> try to do what the libass-wiki recommends and add an empty command
> block after an escaped opening curly brace. This way VSFilters
> will display a lone backslash instead of a opening curly brace
> but otherwise work fine without swallowing up text.
> If done consistently closing curly braces won't need to be
> escaped at all anymore.
> However, such a VSFilter-compatibility change is unrelated to
> fixing the broken \\ escape which doesn't work anywhere.
see above, I'm not into changing ffmpeg's ass output, it's all
about the internally used ass format and the escaping is
a central problem there.
> (Note that the wordjoiner doesn't have font or spacing issues as
> it’s defined to be invisible and zero-width.
Yes, and the use of this has already created issues, even in libass:
https://github.com/libass/libass/issues/507
So it's very likely to cause issues in other implementations
as well, and not many are developed as actively as libass.
(and even that doesn't help when you don't get an update
for your device/software).
I wouldn't want to close like that, but I'm getting distracted
right now. Will get back later.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft
> Works
> Sent: Friday, February 4, 2022 6:34 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> Soft
> > Works
> > Sent: Friday, February 4, 2022 2:58 AM
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> >
> >
> > > -Original Message-
> > > From: ffmpeg-devel On Behalf Of
> > > Oneric
> > > Sent: Friday, February 4, 2022 2:01 AM
> > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> > fix
> > > handling of backslashes
> > >
> > > On Thu, Feb 03, 2022 at 20:51:16 +, Soft Works wrote:
> > > > I think when you inject that word-joiner as a workaround for ass
> > > > parsing, you'll also need to make sure that it gets removed
> > > > when encoding to other formats.
> > >
> > > There's no way of knowing whether the word-joiner comes from
> > > a conversion performed by ffmpeg in the past or already existed
> > > in the original source.
> >
> > That might be true, but I think it's valid to say that such
> characters
> > are very unusual "original" subtitle sources and that's why I don't
> > think it's a good idea for ffmpeg to start injecting them.
> >
> > Subtitle implementations are often rather minimal, especially in
> > hardware devices and might not always cover the full range of
> > UTF-8 specifics.
> >
> > > However, the wordjoiner does not alter the visually appearance and
> > > is unlikely to change line-breaking properties; that's why I chose
> > > a word-joiner. Therefore I don't think removing (only) the
> inserted
> > > word-joiners is possible,
> >
> > Why not? As it seems to be required for ASS encoding only, all other
> > output formats should remain unaffected.
> >
> > > but also not necessary.
> >
> > I'm not sure whether all ffmpeg text-sub encoders can handle
> > those chars - which could be verified of course.
> >
> > But what remains is the question about the effect on end devices
> > which are consuming that output.
> >
> > Finally, those chars are a pest. I'm using them myself for a
> > specific use case, but when you don't know they are there, it can
> > drive you totally mad, eventually even thinking your system or
> > software is faulty.
> >
> > Example:
> >
> > Open your patch file [2/2] and search for the string
> > "123456\NAscending". You can see the string in two lines, but search
> > will only find one of them.
> >
> > Or just look at the two lines directly. They are preceded by + and -
> > even though both appear identical.
> >
> >
> > So, this also needs consideration of the consequences, like how
> > many developers (inside and outside of ffmpeg) this would be driving
> > nuts over the years and make them start hating ffmpeg for doing so
> > once they've found out.
>
> As I really hate how many devs on this ML keep saying 'no' to
> submitted
> code without having a better suggestion, assuming that this is all
> that
> it takes, I don't want to assimilate in this regard.
>
> Hence I want to propose the following solution:
>
> First of all, the existing code in ff_ass_bprint_text_event() is
> totally
> wrong already. Not only with regard to the backslash escaping (as you
> have already pointed out), but also the curly brace escaping is
> invalid.
> There is no curly-brace escaping in ASS either.
>
> In fact it is impossible with ASS to display an opening curly brace
> followed
> by a closing curly brace at a subsequent position (each one alone may
> work
> depending on implementation).
>
> If it was about ASS alone, we might just drop those braces, so we
> could
> at least avoid the text in-between from being hidden (when outputting
> ASS), but ASS is also the internal ("uncompressed/raw") subtitle
> format
> in ffmpeg that is used for conversion (and subtitle filtering).
> So it would be hard-to-sell when curly braces would get lost when
> converting from one text-sub format to another with none of them
>
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft
> Works
> Sent: Friday, February 4, 2022 6:34 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> Soft
> > Works
> > Sent: Friday, February 4, 2022 2:58 AM
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> >
> >
> > > -Original Message-
> > > From: ffmpeg-devel On Behalf Of
> > > Oneric
> > > Sent: Friday, February 4, 2022 2:01 AM
> > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> > fix
> > > handling of backslashes
> > >
> > > On Thu, Feb 03, 2022 at 20:51:16 +, Soft Works wrote:
[..]
> Needless to say that we will of course change the substituted curly
> braces
> back to the regular ones at the encoding side for all but ASS.
> Remains the question what to do when encoding to ASS: We can either
> keep the alternate brace characters or just remove them (or maybe
> replace
> with square brackets).
>
> I'm not sure about that last point
The reason why I'm saying this is because we can't be sure that the
font being used at the target would include glyphs for those alternate
curly braces, and when it doesn't it would typically cause ugly
replacement glyphs (usually a square outline) to be rendered.
But to set the record straight: we're not talking about a degradation
to current here. Right now this doesn't work at all (text inside
curly braces being invisible)
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft
> Works
> Sent: Friday, February 4, 2022 2:58 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> > Oneric
> > Sent: Friday, February 4, 2022 2:01 AM
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> > On Thu, Feb 03, 2022 at 20:51:16 +, Soft Works wrote:
> > > I think when you inject that word-joiner as a workaround for ass
> > > parsing, you'll also need to make sure that it gets removed
> > > when encoding to other formats.
> >
> > There's no way of knowing whether the word-joiner comes from
> > a conversion performed by ffmpeg in the past or already existed
> > in the original source.
>
> That might be true, but I think it's valid to say that such characters
> are very unusual "original" subtitle sources and that's why I don't
> think it's a good idea for ffmpeg to start injecting them.
>
> Subtitle implementations are often rather minimal, especially in
> hardware devices and might not always cover the full range of
> UTF-8 specifics.
>
> > However, the wordjoiner does not alter the visually appearance and
> > is unlikely to change line-breaking properties; that's why I chose
> > a word-joiner. Therefore I don't think removing (only) the inserted
> > word-joiners is possible,
>
> Why not? As it seems to be required for ASS encoding only, all other
> output formats should remain unaffected.
>
> > but also not necessary.
>
> I'm not sure whether all ffmpeg text-sub encoders can handle
> those chars - which could be verified of course.
>
> But what remains is the question about the effect on end devices
> which are consuming that output.
>
> Finally, those chars are a pest. I'm using them myself for a
> specific use case, but when you don't know they are there, it can
> drive you totally mad, eventually even thinking your system or
> software is faulty.
>
> Example:
>
> Open your patch file [2/2] and search for the string
> "123456\NAscending". You can see the string in two lines, but search
> will only find one of them.
>
> Or just look at the two lines directly. They are preceded by + and -
> even though both appear identical.
>
>
> So, this also needs consideration of the consequences, like how
> many developers (inside and outside of ffmpeg) this would be driving
> nuts over the years and make them start hating ffmpeg for doing so
> once they've found out.
As I really hate how many devs on this ML keep saying 'no' to submitted
code without having a better suggestion, assuming that this is all that
it takes, I don't want to assimilate in this regard.
Hence I want to propose the following solution:
First of all, the existing code in ff_ass_bprint_text_event() is totally
wrong already. Not only with regard to the backslash escaping (as you
have already pointed out), but also the curly brace escaping is invalid.
There is no curly-brace escaping in ASS either.
In fact it is impossible with ASS to display an opening curly brace followed
by a closing curly brace at a subsequent position (each one alone may work
depending on implementation).
If it was about ASS alone, we might just drop those braces, so we could
at least avoid the text in-between from being hidden (when outputting
ASS), but ASS is also the internal ("uncompressed/raw") subtitle format
in ffmpeg that is used for conversion (and subtitle filtering).
So it would be hard-to-sell when curly braces would get lost when
converting from one text-sub format to another with none of them
even being ASS.
What we need is to stop creating invalid ASS and at the same time
ensure proper conversion of curly braces. How? We substitute them!
And still, UTF-8 can come to the rescue. There are two suitable
candidates for that:
SMALL LEFT CURLY BRACKET (U+FE5B, Ps): ﹛
SMALL RIGHT CURLY BRACKET (U+FE5C, Pe): ﹜
FULLWIDTH LEFT CURLY BRACKET (U+FF5B, Ps): ｛
FULLWIDTH RIGHT CURLY BRACKET (U+FF5D, Pe): ｝
Substitution of curly braces with one of those will prevent ASS from treating
any possible subtitle content as override code.
What remains to be handled now is the backslash case. Now that we can be sure
that we are never inside a sequence that ASS would consider an override code,
only 3 cases are remaining where the backslash has a meaning in ASS dialog
text: '\n', '\N' and '\h'.
We can simply escape those sequences by in
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Friday, February 4, 2022 2:01 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Thu, Feb 03, 2022 at 20:51:16 +, Soft Works wrote:
> > I think when you inject that word-joiner as a workaround for ass
> > parsing, you'll also need to make sure that it gets removed
> > when encoding to other formats.
>
> There's no way of knowing whether the word-joiner comes from
> a conversion performed by ffmpeg in the past or already existed
> in the original source.
That might be true, but I think it's valid to say that such characters
are very unusual "original" subtitle sources and that's why I don't
think it's a good idea for ffmpeg to start injecting them.
Subtitle implementations are often rather minimal, especially in
hardware devices and might not always cover the full range of
UTF-8 specifics.
> However, the wordjoiner does not alter the visually appearance and
> is unlikely to change line-breaking properties; that's why I chose
> a word-joiner. Therefore I don't think removing (only) the inserted
> word-joiners is possible,
Why not? As it seems to be required for ASS encoding only, all other
output formats should remain unaffected.
> but also not necessary.
I'm not sure whether all ffmpeg text-sub encoders can handle
those chars - which could be verified of course.
But what remains is the question about the effect on end devices
which are consuming that output.
Finally, those chars are a pest. I'm using them myself for a
specific use case, but when you don't know they are there, it can
drive you totally mad, eventually even thinking your system or
software is faulty.
Example:
Open your patch file [2/2] and search for the string
"123456\NAscending". You can see the string in two lines, but search
will only find one of them.
Or just look at the two lines directly. They are preceded by + and -
even though both appear identical.
So, this also needs consideration of the consequences, like how
many developers (inside and outside of ffmpeg) this would be driving
nuts over the years and make them start hating ffmpeg for doing so
once they've found out.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
> -Original Message- > From: ffmpeg-devel On Behalf Of > Scott Theisen > Sent: Friday, February 4, 2022 12:15 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect > yellow appearance of dvd subtitles > > On 2/3/22 17:57, Soft Works wrote: > > My suspicion is that 0x00 was written assuming RGBA order > instead > > of ARGB. > > You are missing a byte for either RGBA or ARGB. RGBA would be cyan. > I think using yellow for debugging purposes is more likely, i.e. it Something made me think that it might have come from a byte ordering issue when I had debugged an example, but I don't remember exactly. Probably you are just right about it, but only Fabrice might be able to tell for sure.. > However, it shouldn't > have been committed like that, if that was the reason. No matter the reason, I don't think there's any doubt that it's wrong? softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Scott Theisen
> Sent: Thursday, February 3, 2022 11:17 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect
> yellow appearance of dvd subtitles
>
> On 2/3/22 17:10, Soft Works wrote:
> >
> >> -Original Message-
> >> From: ffmpegagent
> >> Sent: Tuesday, January 4, 2022 3:19 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: softworkz ; softworkz
> >>
> >> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance
> of
> >> dvd subtitles
> >>
> >> From: softworkz
> >>
> >> The guess_palette() implementation is questionable in itself
> >> as its results don't match those from other DVD subtitle decoders.
> >>
> >> This commit starts cleanup by fixing an obvious bug which has made
> >> certain DVD subs appear yellow instead of white or grey for more
> than
> >> 10 years..
> >>
> >> Signed-off-by: softworkz
> >> ---
> >> avcodec/dvdsubdec: fix incorrect yellow appearance of dvd
> >> subtitles
> >>
> >> Fixes an age-old bug in decoding DVD subtitles.
> >>
> >> Ever wondered why certain DVD subtitles are shown in yellow
> color
> >> when
> >> ffmpeg is involved...
> >>
> >> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> >> ffstaging-16%2Fsoftworkz%2Fpatch_dvdsubdec_fix-v1
> >> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> >> ffstaging-16/softworkz/patch_dvdsubdec_fix-v1
> >> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/16
> >>
> >> libavcodec/dvdsubdec.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> >> index 52259f0730..a3fdb535a5 100644
> >> --- a/libavcodec/dvdsubdec.c
> >> +++ b/libavcodec/dvdsubdec.c
> >> @@ -400,7 +400,7 @@ static int decode_dvd_subtitles(DVDSubContext
> >> *ctx, AVSubtitle *sub_header,
> >> } else {
> >> sub_header->rects[0]->nb_colors = 4;
> >> guess_palette(ctx, (uint32_t*)sub_header-
> >>> rects[0]->data[1],
> >> - 0x00);
> >> + 0xff);
> >> }
> >> sub_header->rects[0]->x = x1;
> >> sub_header->rects[0]->y = y1;
> >>
> >> base-commit: 573b6b8a607398c5f34108efda9c29d41c5727ff
> >> --
> >> ffmpeg-codebot
> > Ping. (no maintainer seems to be registered for this)
>
> MythTV has used this fix since 2010-06-04. See
> https://github.com/ulmus-
> scott/FFmpeg/commit/e2b1a6ee63c01ac9c82d24e2e6cfffeb2bfc
>
> libavcodec/dvdsubdec.c: default to white instead of yellow
>
> from Improved display of DVD subtitles in containers other than the
> original.
> https://github.com/MythTV/mythtv/commit/2510a0821ea4453eb9b34dd96e68ff
> 0441459d0b
> references: https://code.mythtv.org/trac/ticket/8222
Thanks for the references. I have compared the ffmpeg output
with a range of (non-ffmpeg-based) players including VLC,
an example where I had a screen photo from a hardware player
and two subtitle editing tools.
None of those showed them yellow.
> For whatever strange reason, ffmpeg has always used yellow:
> https://github.com/FFmpeg/FFmpeg/commit/240c1657dcd45adc0e63ef947b9209
> 19071ec1f7
My suspicion is that 0x00 was written assuming RGBA order instead
of ARGB.
I'm pretty sure that it's a mistake and it hasn't been an intentional
choice, because even when you would consider a yellow color to for
whatever reason, it wouldn't have been exactly this yellow (#ff0),
because its lightness/saturation values are not in a range of what
is suitable for colored subtitles.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles
> -Original Message-
> From: ffmpegagent
> Sent: Tuesday, January 4, 2022 3:19 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: softworkz ; softworkz
>
> Subject: [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of
> dvd subtitles
>
> From: softworkz
>
> The guess_palette() implementation is questionable in itself
> as its results don't match those from other DVD subtitle decoders.
>
> This commit starts cleanup by fixing an obvious bug which has made
> certain DVD subs appear yellow instead of white or grey for more than
> 10 years..
>
> Signed-off-by: softworkz
> ---
> avcodec/dvdsubdec: fix incorrect yellow appearance of dvd
> subtitles
>
> Fixes an age-old bug in decoding DVD subtitles.
>
> Ever wondered why certain DVD subtitles are shown in yellow color
> when
> ffmpeg is involved...
>
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-16%2Fsoftworkz%2Fpatch_dvdsubdec_fix-v1
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-16/softworkz/patch_dvdsubdec_fix-v1
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/16
>
> libavcodec/dvdsubdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index 52259f0730..a3fdb535a5 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -400,7 +400,7 @@ static int decode_dvd_subtitles(DVDSubContext
> *ctx, AVSubtitle *sub_header,
> } else {
> sub_header->rects[0]->nb_colors = 4;
> guess_palette(ctx, (uint32_t*)sub_header-
> >rects[0]->data[1],
> - 0x00);
> + 0xff);
> }
> sub_header->rects[0]->x = x1;
> sub_header->rects[0]->y = y1;
>
> base-commit: 573b6b8a607398c5f34108efda9c29d41c5727ff
> --
> ffmpeg-codebot
Ping. (no maintainer seems to be registered for this)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 0/2] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
> -Original Message-
> From: ffmpegagent
> Sent: Monday, January 10, 2022 8:56 PM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Hendrik Leppkes ; Soft Works
> ; Marton Balint ; softworkz
>
> Subject: [PATCH v2 0/2] avcodec/dvbsubdec,dvdsubdec: don't dump images
> to disk based on DEBUG define
>
> It's annoying and unexpected, but still useful at times (as I've
> realized
> just recently).
>
> This is a follow-up to the earlier submission here:
> https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html
>
> There has been a comment from Anton, questioning whether the dump-
> feature is
> useful. Meanwhile I came to the conclusion that it can be useful in-
> fact. It
> just shouldn't happen automatically when DEBUG is defined. That's what
> these
> patches do.
>
> I also added fixes for the fopen() call.
>
> softworkz (2):
> avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based on
> DEBUG
> define
> avcodec/dvdsubdec,dvbsubdec: fix writing ppm
>
> libavcodec/dvbsubdec.c | 20 +---
> libavcodec/dvdsubdec.c | 11 ---
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
>
> base-commit: 6c4074e4234edacfb3f37184fd68771df3cb2b7f
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-17%2Fsoftworkz%2Fsubmit_dvb_subs-v2
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-17/softworkz/submit_dvb_subs-v2
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/17
>
> Range-diff vs v1:
>
> 1: 6ca8905c3d ! 1: 2f12ac7f1f avcodec/dvbsubdec: don't dump images
> to disk based on DEBUG define
> @@ Metadata
> Author: softworkz
>
>## Commit message ##
> -avcodec/dvbsubdec: don't dump images to disk based on DEBUG
> define
> +avcodec/dvdsubdec,dvbsubdec: don't dump images to disk based
> on DEBUG define
>
> It's been a regular annoyance.
> Introduce a debug-only parameter for this.
> @@ libavcodec/dvbsubdec.c: static const AVOption options[] = {
>{NULL}
>};
>static const AVClass dvbsubdec_class = {
> +
> + ## libavcodec/dvdsubdec.c ##
> +@@ libavcodec/dvdsubdec.c: typedef struct DVDSubContext
> + uint8_t used_color[256];
> + #ifdef DEBUG
> + int sub_id;
> ++ int dump_imgs;
> + #endif
> + } DVDSubContext;
> +
> +@@ libavcodec/dvdsubdec.c: static int
> dvdsub_decode(AVCodecContext *avctx,
> + ff_dlog(NULL, "start=%d ms end =%d ms\n",
> + sub->start_display_time,
> + sub->end_display_time);
> +-ppm_save(ppm_name, sub->rects[0]->data[0],
> +- sub->rects[0]->w, sub->rects[0]->h, (uint32_t*)
> sub->rects[0]->data[1]);
> ++if (ctx->dump_imgs)
> ++ppm_save(ppm_name, sub->rects[0]->data[0],
> ++ sub->rects[0]->w, sub->rects[0]->h,
> (uint32_t*) sub->rects[0]->data[1]);
> + }
> + #endif
> +
> +@@ libavcodec/dvdsubdec.c: static const AVOption options[] = {
> + { "palette", "set the global palette", OFFSET(palette_str),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD },
> + { "ifo_palette", "obtain the global palette from .IFO
> file", OFFSET(ifo_str), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, SD
> },
> + { "forced_subs_only", "Only show forced subtitles",
> OFFSET(forced_subs_only), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SD},
> ++#ifdef DEBUG
> ++{ "dump_imgs", "Dump subtitle images to disk",
> OFFSET(dump_imgs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, SD},
> ++#endif
> + { NULL }
> + };
> + static const AVClass dvdsub_class = {
> 2: 8da6e4ab17 ! 2: 0cd2c77f31 avcodec/dvbsubdec: fix writing ppm
> @@ Metadata
> Author: softworkz
>
>## Commit message ##
> -avcodec/dvbsubdec: fix writing ppm
> +avcodec/dvdsubdec,dvbsubdec: fix writing ppm
>
> fopen needs (b)inary mode
>
> @@ libavcodec/dvbsubdec.c: static void png_save(DVBSubContext
> *ctx, const char *fil
>if (!f) {
>perror(fname2);
>return;
> +
> + ## libavcodec/dvdsubdec.c ##
> +@@ libavcodec/dvdsubdec.c: static void ppm_save(const char
> *filename, uint8_t *bitmap, int w, int h,
> + int back[3] = {0, 255, 0}; /* green background */
> + FILE *f;
> +
> +-f
Re: [FFmpeg-devel] [PATCH] libavutil/tests/md5: Avoid warnings
> -Original Message-
> From: ffmpeg-devel On Behalf Of Carl
> Eugen Hoyos
> Sent: Sunday, January 23, 2022 12:23 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] libavutil/tests/md5: Avoid
> warnings
>
> Am Fr., 21. Jan. 2022 um 02:18 Uhr schrieb softworkz
> :
> >
> > From: softworkz
> >
> > Those are always showing up on Patchwork
> > when FATE tests are failing, covering
> > some possibly more useful information.
> >
> > Signed-off-by: softworkz
> > ---
> > libavutil/tests/md5: Avoid warnings
> >
> > Those are always showing up on Patchwork when FATE tests are
> failing,
> > covering some possibly more useful information.
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-20%2Fsoftworkz%2Fsubmit_md5-v1
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-20/softworkz/submit_md5-v1
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/20
> >
> > libavutil/tests/md5.c | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/tests/md5.c b/libavutil/tests/md5.c
> > index 42e4538e0a..1f27c40987 100644
> > --- a/libavutil/tests/md5.c
> > +++ b/libavutil/tests/md5.c
> > @@ -33,8 +33,12 @@ int main(void)
> > {
> > uint8_t md5val[16];
> > int i;
> > +
> > +#if defined(__clang__) && defined(__clang_major__) &&
> __clang_major__ < 4
> > volatile uint8_t in[1000]; // volatile to workaround
> http://llvm.org/bugs/show_bug.cgi?id=20849
> > -// FIXME remove volatile once it has been fixed and all fate
> clients are updated
>
> I wonder if the volatile shouldn't just be removed after eight years.
>
> Carl Eugen
So, shall I submit another version that simply removes 'volatile'?
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Thursday, February 3, 2022 3:11 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Wed, Feb 02, 2022 at 22:44:18 +, Soft Works wrote:
> > > Sent: Wednesday, February 2, 2022 11:19 PM
> > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org>
> > >
> > > [claims about git and smtp: part 1]
> > >
> >
> > [claims about git and smtp: part 2]
> >
> I’ll ignore all further offtopic messages here.
Aha, suddenly it's off-topic, but ok.
> please actually discuss the content of the patches in this thread.
I think when you inject that word-joiner as a workaround for ass
parsing, you'll also need to make sure that it gets removed
when encoding to other formats.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft
> Works
> Sent: Wednesday, February 2, 2022 11:19 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of
> > Oneric
> > Sent: Wednesday, February 2, 2022 6:03 PM
> > To: FFmpeg development discussions and patches > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> > It appears my previous mail has been interpreted as some sort of
> > attack
> > against you; this is not the case. I am sorry for creating such a
> > misunderstanding.
>
> Don't know who said that, but I don't feel attacked. It's about
> technical
> details and opinions about it. All good.
>
> > The mail was merely meant to clear up the misconception that it
> would
> > be
> > impossible to send mixed line-endings via mail due to some SMTP
> > property
> > using some illustrative examples from the current thread.
> > (I have no idea how SMTP works in detail)
>
> Where did you see such misconception? I think almost everybody knows
> that this is doable over SMTP in some way, and there are in
> fact many ways to do this, even when it would be as stupid like
> attaching
> as zip archive.
>
> It's never been a matter of SMTP. It's the GIT tooling in the context
> of
> "patch-over-SMTP" transport and the way it is using plain-ascii-text
> e-mails for this purpose. IMO, this is a design flaw or just a bug
> when git-send-email and format-patch are creating output with
> transfer-encodings
> 7bit/8bit but mixed EOLs.
>
> I suppose this is due to the tendency of GIT to consider text file
> basically
> as files with lines of text where the line endings are exchangeable
> and
> rather a platform detail than part of the text file's content.
> And text files with mixed line endings are an accidental side effect
> of
> multi-platform work and the endings can be re-unified at any time.
> That's also the reason why format-patch cannot warn, because
> bazillions
> of code files exist which have mixed EOLs (accidentally and
> exchangeable).
>
> Yes, yes yes - don't need to respond that GIT can handle those cases
> flawlessly and precisely when configured appropriately (and when not
> using the e-mail features). That goes without saying.
> I'm just speculating about history, why it might have happened that
> it doesn't work flawlessly when using e-mail.
>
> Via e-mail, it only works when using base64 encoding like you said,
> but this had only been added at a later time and wasn't part of the
> original functionality.
Haha, I almost forgot to get to my own point..
>From that situation above, I would conclude that at least at an earlier
time, the git developers (when asked about a case like the ffmpeg ref
files) might have argued that such files with mixed EOL where the
individual EOLs also form a relevant part of the
content - are not meant to be handled as text, but as binary
files (either) or at to use binary diffs when being send via e-mail.
.
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Oneric
> Sent: Wednesday, February 2, 2022 6:03 PM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> It appears my previous mail has been interpreted as some sort of
> attack
> against you; this is not the case. I am sorry for creating such a
> misunderstanding.
Don't know who said that, but I don't feel attacked. It's about technical
details and opinions about it. All good.
> The mail was merely meant to clear up the misconception that it would
> be
> impossible to send mixed line-endings via mail due to some SMTP
> property
> using some illustrative examples from the current thread.
> (I have no idea how SMTP works in detail)
Where did you see such misconception? I think almost everybody knows
that this is doable over SMTP in some way, and there are in
fact many ways to do this, even when it would be as stupid like attaching
as zip archive.
It's never been a matter of SMTP. It's the GIT tooling in the context of
"patch-over-SMTP" transport and the way it is using plain-ascii-text
e-mails for this purpose. IMO, this is a design flaw or just a bug
when git-send-email and format-patch are creating output with transfer-encodings
7bit/8bit but mixed EOLs.
I suppose this is due to the tendency of GIT to consider text file basically
as files with lines of text where the line endings are exchangeable and
rather a platform detail than part of the text file's content.
And text files with mixed line endings are an accidental side effect of
multi-platform work and the endings can be re-unified at any time.
That's also the reason why format-patch cannot warn, because bazillions
of code files exist which have mixed EOLs (accidentally and exchangeable).
Yes, yes yes - don't need to respond that GIT can handle those cases
flawlessly and precisely when configured appropriately (and when not
using the e-mail features). That goes without saying.
I'm just speculating about history, why it might have happened that
it doesn't work flawlessly when using e-mail.
Via e-mail, it only works when using base64 encoding like you said,
but this had only been added at a later time and wasn't part of the
original functionality.
Further - I could find no evidence for your claims that base64 encoding
would be chosen automatically depending on the content, neither is it
on by default.
There's an 'auto' mode which chooses between 7bit, 8bit and quoted-printable,
but not base64, which always needs to be specified explicitly (or set in
.gitconfig).
This is the way it is documented and I'm seeing exactly that documented
behavior on both Linux and Windows when testing with a mocked smtp server.
You had contradicted my statement that it would be desirable that format-patch
would be able to do base64 encoding as well, saying that this would be the
task of a mail-client. This is not necessarily the case, though.
format-patch can generate patch e-mails with multipart-mime content, where
the patch content is created as an attachment mime part. format-patch does
that with a transfer-encoding of 8bit, but it can't do it with base64 encoding.
(I'm stripping the rest of your reply as it is all covered above already)
Regards,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Oneric
> Sent: Wednesday, February 2, 2022 12:26 AM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Tue, Feb 01, 2022 at 20:41:37 +, Soft Works wrote:
> > > On Tue, Feb 01, 2022 at 19:44:24 +, Soft Works wrote:
> > > > > On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> > > > >
> > > > > In case anyone is wondering why patchwork fails to apply the second
> patch,
> > > > > this is probably once again because the patch updates one of FATE's
> ASS
> > > > > reference files which use CRLF line-endings.
> > > > > Locally git am applies both without a hitch for me on top of current
> master
> > > > > (and FATE passes after applying each patch).
> > > >
> > > >
> > > > You can add a .gitattributes file to tests/ref/fate/ which includes the
> line
> > > >
> > > > sub-webvtt2 -diff
> > > >
> > > > Then your local git format-patch will create a binary diff for the
> file.
> > >
> > > Thanks for your suggestion. However, a binary diff would look like this
> which
> > > isn't great for seeing what's going on during review:
> > >
> > > [...]
> >
> > Yes, I know, but the question is probably what's more important..
> >
> > ..that it can be applied or that the text is viewable?
>
> It CAN be applied (as I've now written twice) and
> of course I verified this with the mail received from the list.
I meant that it can be applied by everybody, including Patchwork
and Michael.
> If you tried to send a patch with your mail client by pasting in the diff,
> then yes, presumably the line-endings would indeed get mangled.
I use git format-patch, just like many others and afaik it can't create base64
encoded content.
BTW. BASE64 doesn't seem to be widely used here, just checked the frequent
committers among the recent submissions. just quickly checked Michael,
Andreas,Gyan,Marton,James,Anton,Derek - all of
> [..] That's why
> doc/developer.texi tells you not to do this (instead recommending
> git send-email or binary (i.e. base64-encoded) attachments).
Interestingly, even the author of those lines is sending patches with
Content-Transfer-Encoding: 7bit :-)
> Tl;dr:
> Mixed line-endings in a diff are no technical issue,
> provided the patch is sent correctly.^
LOL - so the majority on this ML is sending patches incorrectly?
I think, marking such files as binary is a bit more practical than
expecting everybody to change ones working process.
Best,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Oneric
> Sent: Tuesday, February 1, 2022 9:07 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Tue, Feb 01, 2022 at 19:44:24 +, Soft Works wrote:
> > > On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> > >
> > > In case anyone is wondering why patchwork fails to apply the second
> patch,
> > > this is probably once again because the patch updates one of FATE's ASS
> > > reference files which use CRLF line-endings.
> > > Locally git am applies both without a hitch for me on top of current
> master
> > > (and FATE passes after applying each patch).
> >
> >
> > You can add a .gitattributes file to tests/ref/fate/ which includes the
> line
> >
> > sub-webvtt2 -diff
> >
> > Then your local git format-patch will create a binary diff for the file.
>
> Thanks for your suggestion. However, a binary diff would look like this which
> isn't great for seeing what's going on during review:
>
> diff --git a/tests/ref/fate/sub-webvtt2 b/tests/ref/fate/sub-webvtt2
> index
> 357b8178ea1cf224ad47dcf78b24f1948ece6665..4cd1d86a9a58ccf65812131bf84a17531c2
> c6cfa 100644
> GIT binary patch
> delta 24
> gcmeys^NnXiIV;bjhJHgMV-r)eM-6?GYgs=70DwpeRR910
>
> delta 18
> Zcmeyy^MPkWIV+o?k+F%X+2m%{
Yes, I know, but the question is probably what's more important..
..that it can be applied or that the text is viewable?
I assume that a reviewer would often apply the patch locally anyway, then
the text changes become visible again as this doesn't affect git log.
Given that it's just a handful files from a few thousands, this should
be acceptable.
> Also as noted, locally plain `git am` has no issues applying the regular
> (non-binary) patch;
Yes, because it hasn't been sent around via e-mail yet. SMTP requires
CRLF line endings, so essentially it depends on the receiving e-mail client
whether it outputs LF or CRLF. It's then up to git whether it ignores
or adjust the line ending when applying a patch. Maybe that's what is
configured @patchwork, I can't tell.
The most extreme example is sub-textenc (and there was another one iirc).
It has mixed line endings - some LF and some CRLF. At least at that point
there's no way out, because those endings will get unrecoverably lost
as soon as it is sent around via e-mail as text-diff.
That's how I came to the binary diff as only working option.
(or you just don't send patches around via e-mail at all ;-)
> I recall using some .gitattribute settings to force crlf (without making
> the file binary?) were discussed last time this happened, but deemed to be
> not worth it because of some other issues with it.
Doesn't work with mixed endings. BTW, I don't find it wrong to make such files
binary because logically these are not text files, but test reference results,
which are expected to precisely match (rather than "have the same text").
If you don't want to mark the files as binary, you could probably use
.gitattributes without commiting, so it would only affect format-patch
generation.
Best,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> -Original Message-
> From: ffmpeg-devel On Behalf Of Oneric
> Sent: Tuesday, February 1, 2022 6:39 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
>
> On Sun, Jan 16, 2022 at 19:16:54 +0100, Oneric wrote:
> > [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes
> > libavcodec/ass.c | 5 -
> > libavcodec/webvttdec.c | 2 +-
> > 2 files changed, 5 insertions(+), 2 deletions(-)
>
> > [PATCH 2/2] avcodec/webvttdec: honour bidi marks
> > libavcodec/webvttdec.c | 2 +-
> > tests/ref/fate/sub-webvtt2 | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
>
> ping.
>
> In case anyone is wondering why patchwork fails to apply the second patch,
> this is probably once again because the patch updates one of FATE's ASS
> reference files which use CRLF line-endings.
> Locally git am applies both without a hitch for me on top of current master
> (and FATE passes after applying each patch).
You can add a .gitattributes file to tests/ref/fate/ which includes the line
sub-webvtt2 -diff
Then your local git format-patch will create a binary diff for the file.
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Patchwork FATE Errors
> -Original Message- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Friday, January 21, 2022 5:52 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Andriy > > Gelman > > Sent: Friday, January 21, 2022 5:24 AM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > On Fri, 21. Jan 01:44, Soft Works wrote: > > > > > > > > > > -Original Message- > > > > From: ffmpeg-devel On Behalf Of > Andreas > > > > Rheinhardt > > > > Sent: Friday, January 21, 2022 2:31 AM > > > > To: ffmpeg-devel@ffmpeg.org > > > > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > > > > > Soft Works: > > > > > > > > > > > > > > >> -Original Message- > > > > >> From: ffmpeg-devel On Behalf Of > > Andreas > > > > >> Rheinhardt > > > > >> Sent: Thursday, January 20, 2022 11:38 PM > > > > >> To: ffmpeg-devel@ffmpeg.org > > > > >> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > >> > > > > >> Soft Works: > > > > >>> > > > > >>> > > > > >>>> -Original Message- > > > > >>>> From: ffmpeg-devel On Behalf Of > > > > Andreas > > > > >>>> Rheinhardt > > > > >>>> Sent: Thursday, January 20, 2022 10:22 PM > > > > >>>> To: ffmpeg-devel@ffmpeg.org > > > > >>>> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > >>>> > > > > >>>> Soft Works: > > > > >>>>> > > > > >>>>> andriy/make_fate_ppc > > > > >>>>> > > > > >>>>> => Does it possibly need 'make fate-rsync'? > > > > >>>>> > > > > >>>> > > > > >>>> No. The test does not rely on need samples; > > > > >>> > > > > >>> It was just a very quick guess, because yesterday I rebased and > > > > >>> saw the test matroska-dovi-write-config7 failing which was fixed > > > > >>> after fate-rsync - that's why I though it might be the same reason > > > > >>> (with make -jX, it's probably not deterministic, which test will > > > > >>> fail first). > > > > >>> > > > > >>> > > > > >>>> and the other test that uses > > > > >>>> this sample works fine. Some time ago, someone else wrote FATE > tests > > for > > > > >>>> AVDOVIDecoderConfigurationRecord in Matroska > > > > >>>> > > > > > (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729- > > > > >> 6- > > > > >>>> tcchlis...@gmail.com/). > > > > >>>> These were faulty and one of them relied on a sample that has > > apparently > > > > >>>> never been uploaded (but this test is actually redundant with the > > other > > > > >>>> test), so I investigated and saw that the test (presumably > > > > >>>> unintentially) reencoded audio, so I switched it to a pure copy > test > > and > > > > >>>> applied it, believing that codec-copy tests could not possibly for > > some > > > > >>>> arches. That was a mistake and I am deeply sorry for this mess. > > > > >>> > > > > >>> Nevermind - things happen.. > > > > >>> > > > > >>> > > > > >>> BTW, I was thinking about submitting a patch for > > libavutil/tests/md5.c > > > > >>> > > > > >>> something like: > > > > >>> > > > > >>> #ifdef __GNUC__ > > > > >>> #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" > > > > >>> #endif > > > > >>> > > > > >>> #ifdef __clang__ > > > > >>> #pragma clang diagnostic ignored "-Wdiscarded-qualifiers" > > > > >>> #end
Re: [FFmpeg-devel] Patchwork FATE Errors
> -Original Message- > From: ffmpeg-devel On Behalf Of Andriy > Gelman > Sent: Friday, January 21, 2022 5:24 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > On Fri, 21. Jan 01:44, Soft Works wrote: > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Andreas > > > Rheinhardt > > > Sent: Friday, January 21, 2022 2:31 AM > > > To: ffmpeg-devel@ffmpeg.org > > > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > > > > Soft Works: > > > > > > > > > > > >> -Original Message- > > > >> From: ffmpeg-devel On Behalf Of > Andreas > > > >> Rheinhardt > > > >> Sent: Thursday, January 20, 2022 11:38 PM > > > >> To: ffmpeg-devel@ffmpeg.org > > > >> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > >> > > > >> Soft Works: > > > >>> > > > >>> > > > >>>> -Original Message- > > > >>>> From: ffmpeg-devel On Behalf Of > > > Andreas > > > >>>> Rheinhardt > > > >>>> Sent: Thursday, January 20, 2022 10:22 PM > > > >>>> To: ffmpeg-devel@ffmpeg.org > > > >>>> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > > >>>> > > > >>>> Soft Works: > > > >>>>> > > > >>>>> andriy/make_fate_ppc > > > >>>>> > > > >>>>> => Does it possibly need 'make fate-rsync'? > > > >>>>> > > > >>>> > > > >>>> No. The test does not rely on need samples; > > > >>> > > > >>> It was just a very quick guess, because yesterday I rebased and > > > >>> saw the test matroska-dovi-write-config7 failing which was fixed > > > >>> after fate-rsync - that's why I though it might be the same reason > > > >>> (with make -jX, it's probably not deterministic, which test will > > > >>> fail first). > > > >>> > > > >>> > > > >>>> and the other test that uses > > > >>>> this sample works fine. Some time ago, someone else wrote FATE tests > for > > > >>>> AVDOVIDecoderConfigurationRecord in Matroska > > > >>>> > > > (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729- > > > >> 6- > > > >>>> tcchlis...@gmail.com/). > > > >>>> These were faulty and one of them relied on a sample that has > apparently > > > >>>> never been uploaded (but this test is actually redundant with the > other > > > >>>> test), so I investigated and saw that the test (presumably > > > >>>> unintentially) reencoded audio, so I switched it to a pure copy test > and > > > >>>> applied it, believing that codec-copy tests could not possibly for > some > > > >>>> arches. That was a mistake and I am deeply sorry for this mess. > > > >>> > > > >>> Nevermind - things happen.. > > > >>> > > > >>> > > > >>> BTW, I was thinking about submitting a patch for > libavutil/tests/md5.c > > > >>> > > > >>> something like: > > > >>> > > > >>> #ifdef __GNUC__ > > > >>> #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" > > > >>> #endif > > > >>> > > > >>> #ifdef __clang__ > > > >>> #pragma clang diagnostic ignored "-Wdiscarded-qualifiers" > > > >>> #endif > > > >>> > > > >>> Would that make sense? > > > >>> Those warnings are appearing in every single fate error output on > > > >> patchwork, > > > >>> possibly covering up more relevant things. > > > >>> > > > >> > > > >> Instead of pragmas one should limit the volatile to those compilers > > > >> which miscompile the code without them. > > > >> (IMO one does not need to find the exact set of compilers that > > > >> miscompile this; all that matters is that recent versions don't give > > > >> warnings and old versi
Re: [FFmpeg-devel] [PATCH v2] libavutil/tests/md5: Avoid warnings
> -Original Message-
> From: ffmpeg-devel On Behalf Of James Almer
> Sent: Friday, January 21, 2022 2:45 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavutil/tests/md5: Avoid warnings
>
>
>
> On 1/20/2022 10:41 PM, softworkz wrote:
> > From: softworkz
> >
> > Those are always showing up on Patchwork
> > when FATE tests are failing, covering
> > some possibly more useful information.
> >
> > Signed-off-by: softworkz
> > ---
> > libavutil/tests/md5: Avoid warnings
> >
> > Those are always showing up on Patchwork when FATE tests are failing,
> > covering some possibly more useful information.
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-20%2Fsoftworkz%2Fsubmit_md5-v2
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> 20/softworkz/submit_md5-v2
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/20
> >
> > Range-diff vs v1:
> >
> > 1: 3910d81fa1 ! 1: 4723054463 libavutil/tests/md5: Avoid warnings
> > @@ libavutil/tests/md5.c: int main(void)
> > {
> > uint8_t md5val[16];
> > int i;
> > -+
> > -+#if defined(__clang__) && defined(__clang_major__) &&
> __clang_major__ < 4
> > - volatile uint8_t in[1000]; // volatile to workaround
> http://llvm.org/bugs/show_bug.cgi?id=20849
> > +-volatile uint8_t in[1000]; // volatile to workaround
> http://llvm.org/bugs/show_bug.cgi?id=20849
> >-// FIXME remove volatile once it has been fixed and all fate
> clients are updated
> > -+#else
> > -+uint8_t in[1000];
> > ++
> > ++#if 1 || defined(__clang__) && defined(__clang_major__) &&
> __clang_major__ < 4
> > ++volatile // volatile to workaround
> http://llvm.org/bugs/show_bug.cgi?id=20849
> >+#endif
> > ++uint8_t in[1000];
> >
> > for (i = 0; i < 1000; i++)
> > in[i] = i * i;
> >
> >
> > libavutil/tests/md5.c | 7 +--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavutil/tests/md5.c b/libavutil/tests/md5.c
> > index 42e4538e0a..cca63d6507 100644
> > --- a/libavutil/tests/md5.c
> > +++ b/libavutil/tests/md5.c
> > @@ -33,8 +33,11 @@ int main(void)
> > {
> > uint8_t md5val[16];
> > int i;
> > -volatile uint8_t in[1000]; // volatile to workaround
> http://llvm.org/bugs/show_bug.cgi?id=20849
> > -// FIXME remove volatile once it has been fixed and all fate clients
> are updated
> > +
> > +#if 1 || defined(__clang__) && defined(__clang_major__) && __clang_major__
> < 4
>
> 1?
I'm an idiot...
Thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Patchwork FATE Errors
> -Original Message- > From: ffmpeg-devel On Behalf Of Andreas > Rheinhardt > Sent: Friday, January 21, 2022 2:31 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > Soft Works: > > > > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Andreas > >> Rheinhardt > >> Sent: Thursday, January 20, 2022 11:38 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > >> > >> Soft Works: > >>> > >>> > >>>> -Original Message- > >>>> From: ffmpeg-devel On Behalf Of > Andreas > >>>> Rheinhardt > >>>> Sent: Thursday, January 20, 2022 10:22 PM > >>>> To: ffmpeg-devel@ffmpeg.org > >>>> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > >>>> > >>>> Soft Works: > >>>>> > >>>>> andriy/make_fate_ppc > >>>>> > >>>>> => Does it possibly need 'make fate-rsync'? > >>>>> > >>>> > >>>> No. The test does not rely on need samples; > >>> > >>> It was just a very quick guess, because yesterday I rebased and > >>> saw the test matroska-dovi-write-config7 failing which was fixed > >>> after fate-rsync - that's why I though it might be the same reason > >>> (with make -jX, it's probably not deterministic, which test will > >>> fail first). > >>> > >>> > >>>> and the other test that uses > >>>> this sample works fine. Some time ago, someone else wrote FATE tests for > >>>> AVDOVIDecoderConfigurationRecord in Matroska > >>>> > (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729- > >> 6- > >>>> tcchlis...@gmail.com/). > >>>> These were faulty and one of them relied on a sample that has apparently > >>>> never been uploaded (but this test is actually redundant with the other > >>>> test), so I investigated and saw that the test (presumably > >>>> unintentially) reencoded audio, so I switched it to a pure copy test and > >>>> applied it, believing that codec-copy tests could not possibly for some > >>>> arches. That was a mistake and I am deeply sorry for this mess. > >>> > >>> Nevermind - things happen.. > >>> > >>> > >>> BTW, I was thinking about submitting a patch for libavutil/tests/md5.c > >>> > >>> something like: > >>> > >>> #ifdef __GNUC__ > >>> #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" > >>> #endif > >>> > >>> #ifdef __clang__ > >>> #pragma clang diagnostic ignored "-Wdiscarded-qualifiers" > >>> #endif > >>> > >>> Would that make sense? > >>> Those warnings are appearing in every single fate error output on > >> patchwork, > >>> possibly covering up more relevant things. > >>> > >> > >> Instead of pragmas one should limit the volatile to those compilers > >> which miscompile the code without them. > >> (IMO one does not need to find the exact set of compilers that > >> miscompile this; all that matters is that recent versions don't give > >> warnings and old versions don't miscompile. If some compilers of medium > >> age still show this warning afterwards without needing the volatile, so > >> be it.) > > > > You mean like this? > > > > #if defined(__clang__) && defined(__clang_major__) && __clang_major__ < 4 > > volatile uint8_t in[1000]; // volatile to workaround > http://llvm.org/bugs/show_bug.cgi?id=20849 > > #else > > uint8_t in[1000]; > > #endif > > > > I would not use an else branch, but only put the volatile and the > comment in the #if branch. > > > > > It was fixed in 3.5.1, so "medium age" would be 3.5.1 to 4.0.0 > > > > Fine by me if tested. I do not have a Clang setup locally, I'd assume at least one of the Patchwork VMs uses Clang? softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Sunday, January 2, 2022 7:16 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] GitHub Integration > > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of Lynne > > Sent: Sunday, January 2, 2022 3:04 PM > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] GitHub Integration > > > > 2 Jan 2022, 04:28 by softwo...@hotmail.com: > > > > > > > > > > >> -Original Message- > > >> From: ffmpeg-devel On Behalf Of Soft > > Works > > >> Sent: Tuesday, December 28, 2021 10:58 PM > > >> To: FFmpeg development discussions and patches > > >> Subject: Re: [FFmpeg-devel] GitHub Integration > > >> > > >> One component of the GitHub "Bridge" is a mirror of the ffmpeg-devel > > >> mailing list as a Git repository which provides a web interfaces, can be > > >> cloned via Git and can also be accessed from Atom feed readers. > > >> This allows to follow the mailing list without subscribing. The web UI > > >> might not be everybody's taste, though. It's the same that is used for > > >> Linux kernel mailing lists (https://lore.kernel.org/git/). > > >> I had to set this up as it's a requirement for the GitHub "Bridge", but > > >> maybe it's useful for someone in other ways: > > >> > > >> https://master.gitmailbox.com/ffmpegdev/ > > >> > > > > > > > > > I have just ACTIVATED patch submission via GitHub and submitted the first > > > patch through this method. > > > > > > Pull requests can be created in this repository (for now): > > > > > > https://github.com/ffstaging/FFmpeg > > > > > > > > > What's not nice is that the submitted e-mail shows "ffmpegagent" as > sender > > > and patchwork is showing it under that name as well. > > > > > > We'll need to see how this can be improved. > > > > > > > I don't like it. Can't it use the author's name for emails? > > I don't like this either. I'll try to find a way. This is fixed now. See my patch from today: [FFmpeg-devel] libavutil/tests/md5: Avoid warnings Also, it appears correctly attributed on Patchwork: (no longer as ffmpegagent) https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.20.ffstaging.ffmpeg.1642727870274.ffmpegag...@gmail.com/ Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Patchwork FATE Errors
> -Original Message- > From: ffmpeg-devel On Behalf Of Andreas > Rheinhardt > Sent: Thursday, January 20, 2022 11:38 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > Soft Works: > > > > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Andreas > >> Rheinhardt > >> Sent: Thursday, January 20, 2022 10:22 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > >> > >> Soft Works: > >>> > >>> andriy/make_fate_ppc > >>> > >>> => Does it possibly need 'make fate-rsync'? > >>> > >> > >> No. The test does not rely on need samples; > > > > It was just a very quick guess, because yesterday I rebased and > > saw the test matroska-dovi-write-config7 failing which was fixed > > after fate-rsync - that's why I though it might be the same reason > > (with make -jX, it's probably not deterministic, which test will > > fail first). > > > > > >> and the other test that uses > >> this sample works fine. Some time ago, someone else wrote FATE tests for > >> AVDOVIDecoderConfigurationRecord in Matroska > >> (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729- > 6- > >> tcchlis...@gmail.com/). > >> These were faulty and one of them relied on a sample that has apparently > >> never been uploaded (but this test is actually redundant with the other > >> test), so I investigated and saw that the test (presumably > >> unintentially) reencoded audio, so I switched it to a pure copy test and > >> applied it, believing that codec-copy tests could not possibly for some > >> arches. That was a mistake and I am deeply sorry for this mess. > > > > Nevermind - things happen.. > > > > > > BTW, I was thinking about submitting a patch for libavutil/tests/md5.c > > > > something like: > > > > #ifdef __GNUC__ > > #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" > > #endif > > > > #ifdef __clang__ > > #pragma clang diagnostic ignored "-Wdiscarded-qualifiers" > > #endif > > > > Would that make sense? > > Those warnings are appearing in every single fate error output on > patchwork, > > possibly covering up more relevant things. > > > > Instead of pragmas one should limit the volatile to those compilers > which miscompile the code without them. > (IMO one does not need to find the exact set of compilers that > miscompile this; all that matters is that recent versions don't give > warnings and old versions don't miscompile. If some compilers of medium > age still show this warning afterwards without needing the volatile, so > be it.) You mean like this? #if defined(__clang__) && defined(__clang_major__) && __clang_major__ < 4 volatile uint8_t in[1000]; // volatile to workaround http://llvm.org/bugs/show_bug.cgi?id=20849 #else uint8_t in[1000]; #endif It was fixed in 3.5.1, so "medium age" would be 3.5.1 to 4.0.0 sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Patchwork FATE Errors
> -Original Message- > From: ffmpeg-devel On Behalf Of Andreas > Rheinhardt > Sent: Thursday, January 20, 2022 10:22 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] Patchwork FATE Errors > > Soft Works: > > > > andriy/make_fate_ppc > > > > => Does it possibly need 'make fate-rsync'? > > > > No. The test does not rely on need samples; It was just a very quick guess, because yesterday I rebased and saw the test matroska-dovi-write-config7 failing which was fixed after fate-rsync - that's why I though it might be the same reason (with make -jX, it's probably not deterministic, which test will fail first). > and the other test that uses > this sample works fine. Some time ago, someone else wrote FATE tests for > AVDOVIDecoderConfigurationRecord in Matroska > (https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220101165153.440729-6- > tcchlis...@gmail.com/). > These were faulty and one of them relied on a sample that has apparently > never been uploaded (but this test is actually redundant with the other > test), so I investigated and saw that the test (presumably > unintentially) reencoded audio, so I switched it to a pure copy test and > applied it, believing that codec-copy tests could not possibly for some > arches. That was a mistake and I am deeply sorry for this mess. Nevermind - things happen.. BTW, I was thinking about submitting a patch for libavutil/tests/md5.c something like: #ifdef __GNUC__ #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" #endif #ifdef __clang__ #pragma clang diagnostic ignored "-Wdiscarded-qualifiers" #endif Would that make sense? Those warnings are appearing in every single fate error output on patchwork, possibly covering up more relevant things. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] Patchwork FATE Errors
andriy/make_fate_ppc => Does it possibly need 'make fate-rsync'? softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 21/26] fftools/ffmpeg: Introduce subtitle filtering and new frame-based subtitle encoding
> -Original Message- > From: ffmpeg-devel On Behalf Of Andriy > Gelman > Sent: Thursday, January 20, 2022 7:01 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH v3 21/26] fftools/ffmpeg: Introduce > subtitle filtering and new frame-based subtitle encoding > > Hi, > > On Thu, 20. Jan 15:17, Soft Works wrote: > > > > > > > Patches 19,20,21,22 and 23 do not appear on Patchwork. > > > > Do you have an idea why? > > The full set is on patchwork now. Thanks Andriy! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 21/26] fftools/ffmpeg: Introduce subtitle filtering and new frame-based subtitle encoding
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Thursday, January 20, 2022 11:07 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH v3 21/26] fftools/ffmpeg: Introduce > subtitle filtering and new frame-based subtitle encoding > > On Thu, Jan 20, 2022 at 03:25:29AM +, ffmpegagent wrote: > > From: softworkz [..] > > --- > > fftools/ffmpeg.c | 501 - > > fftools/ffmpeg.h | 13 +- > > fftools/ffmpeg_filter.c | 235 ++-- > > fftools/ffmpeg_hw.c |2 +- > > fftools/ffmpeg_opt.c |3 +- > > tests/ref/fate/filter-overlay-dvdsub-2397 | 182 +-- > > tests/ref/fate/sub-dvb| 162 +-- > > tests/ref/fate/sub2video | 1091 +- > > tests/ref/fate/sub2video_basic| 1239 +++-- > > tests/ref/fate/sub2video_time_limited | 78 +- > > 10 files changed, 2837 insertions(+), 669 deletions(-) > > segfaults > > Stream mapping: > Stream #0:2 -> #0:0 (ass (ssa) -> subrip (srt)) > Press [q] to stop, [?] for help > ==4485== Invalid read of size 8 > ==4485==at 0x3052F7: transcode (in /home/michael/ffmpeg- > git/ffmpeg/ffmpeg_g) > ==4485==by 0x2DDCCB: main (in /home/michael/ffmpeg-git/ffmpeg/ffmpeg_g) > ==4485== Address 0x10 is not stack'd, malloc'd or (recently) free'd > ==4485== > ==4485== > ==4485== Process terminating with default action of signal 11 (SIGSEGV) > ==4485== Access not within mapped region at address 0x10 > ==4485==at 0x3052F7: transcode (in /home/michael/ffmpeg- > git/ffmpeg/ffmpeg_g) > ==4485==by 0x2DDCCB: main (in /home/michael/ffmpeg-git/ffmpeg/ffmpeg_g) > ==4485== If you believe this happened as a result of a stack > ==4485== overflow in your program's main thread (unlikely but > ==4485== possible), you can try to increase the size of the > ==4485== main thread stack using the --main-stacksize= flag. > ==4485== The main thread stack size used in this run was 8388608. > > > will send you the sample privatly Great, thanks! Patches 19,20,21,22 and 23 do not appear on Patchwork. Do you have an idea why? sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: add tile encoding support for VP9
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> myp...@gmail.com
> Sent: Monday, January 17, 2022 11:36 AM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH v2] lavc/qsvenc: add tile encoding support
> for VP9
>
> On Mon, Jan 17, 2022 at 4:30 PM Xiang, Haihao
> wrote:
> >
> > On Thu, 2022-01-13 at 13:45 +0800, Haihao Xiang wrote:
> > > Add -tile_rows and -tile_cols options to specify the number of tile
> > > rows and columns
> > >
> > > Signed-off-by: Haihao Xiang
> > > ---
> > > v2: add option descriptions in the doc
> > >
> > > doc/encoders.texi | 6 ++
> > > libavcodec/qsvenc.c | 4
> > > libavcodec/qsvenc.h | 1 +
> > > libavcodec/qsvenc_vp9.c | 10 ++
> > > 4 files changed, 21 insertions(+)
> > >
> > > diff --git a/doc/encoders.texi b/doc/encoders.texi
> > > index 7cc8be1209..a4176089d5 100644
> > > --- a/doc/encoders.texi
> > > +++ b/doc/encoders.texi
> > > @@ -3457,6 +3457,12 @@ These options are used by vp9_qsv
> > > @item profile2
> > > @item profile3
> > > @end table
> > > +
> > > +@item @var{tile_cols}
> > > +Number of columns for tiled encoding (requires libmfx >= 1.29).
> > > +
> > > +@item @var{tile_rows}
> > > +Number of rows for tiled encoding (requires libmfx >= 1.29).
> > > @end table
> > >
> > > @section snow
> > > diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
> > > index 4e7a15f060..4cbc9ff4dc 100644
> > > --- a/libavcodec/qsvenc.c
> > > +++ b/libavcodec/qsvenc.c
> > > @@ -939,6 +939,10 @@ static int init_video_param(AVCodecContext *avctx,
> > > QSVEncContext *q)
> > > q->extvp9param.Header.BufferId = MFX_EXTBUFF_VP9_PARAM;
> > > q->extvp9param.Header.BufferSz = sizeof(q->extvp9param);
> > > q->extvp9param.WriteIVFHeaders = MFX_CODINGOPTION_OFF;
> > > +#if QSV_HAVE_EXT_VP9_TILES
> > > +q->extvp9param.NumTileColumns = q->tile_cols;
> > > +q->extvp9param.NumTileRows = q->tile_rows;
> > > +#endif
> > > q->extparam_internal[q->nb_extparam_internal++] = (mfxExtBuffer
> *)
> > > >extvp9param;
> > > }
> > > #endif
> > > diff --git a/libavcodec/qsvenc.h b/libavcodec/qsvenc.h
> > > index 31516b8e55..00ee52a5d1 100644
> > > --- a/libavcodec/qsvenc.h
> > > +++ b/libavcodec/qsvenc.h
> > > @@ -41,6 +41,7 @@
> > >
> > > #define QSV_HAVE_EXT_HEVC_TILES QSV_VERSION_ATLEAST(1, 13)
> > > #define QSV_HAVE_EXT_VP9_PARAM QSV_VERSION_ATLEAST(1, 26)
> > > +#define QSV_HAVE_EXT_VP9_TILES QSV_VERSION_ATLEAST(1, 29)
> > >
> > > #define QSV_HAVE_TRELLIS QSV_VERSION_ATLEAST(1, 8)
> > > #define QSV_HAVE_MAX_SLICE_SIZE QSV_VERSION_ATLEAST(1, 9)
> > > diff --git a/libavcodec/qsvenc_vp9.c b/libavcodec/qsvenc_vp9.c
> > > index 9329990d11..1168ddda0e 100644
> > > --- a/libavcodec/qsvenc_vp9.c
> > > +++ b/libavcodec/qsvenc_vp9.c
> > > @@ -73,6 +73,16 @@ static const AVOption options[] = {
> > > { "profile2", NULL, 0, AV_OPT_TYPE_CONST, { .i64
> =
> > > MFX_PROFILE_VP9_2 }, INT_MIN, INT_MAX, VE, "profile" },
> > > { "profile3", NULL, 0, AV_OPT_TYPE_CONST, { .i64
> =
> > > MFX_PROFILE_VP9_3 }, INT_MIN, INT_MAX, VE, "profile" },
> > >
> > > +#if QSV_HAVE_EXT_VP9_TILES
> > > +/* The minimum tile width in luma pixels is 256, set maximum
> tile_cols to
> > > 32 for 8K video */
> > > +{ "tile_cols", "Number of columns for tiled
> > > encoding", OFFSET(qsv.tile_cols),AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
> 32,
> > > VE },
> > > +/* Set maximum tile_rows to 4 per VP9 spec */
> > > +{ "tile_rows", "Number of rows for tiled
> > > encoding", OFFSET(qsv.tile_rows),AV_OPT_TYPE_INT, { .i64 = 0 },
> 0, 4,
> > > VE },
> > > +#else
> > > +{ "tile_cols", "(not
> > > supported)",OFFSET(qsv.tile_cols),
> AV_OPT_TYPE_INT,
> > > { .i64 = 0 }, 0, 0, VE },
> > > +{ "tile_rows", "(not
> > > supported)",OFFSET(qsv.tile_rows),
> AV_OPT_TYPE_INT,
> > > { .i64 = 0 }, 0, 0, VE },
> > > +#endif
> > > +
> perfer one option like "-tile rows x cols" than two options like
The example is invalid. (spaces)
> "-tile_rows row -tile_cols col"
This way, the options are typed, have min, max and default values,
without needing any code to write and maintain.
if you mean "-tile rowsxcols", there's no option type for this,
unless you would want to mis-use the video size type for it.
Otherwise you'd need to make it string and write all the parsing
and checking code for it.
Do you think that it would be worth the effort?
Also, in most use cases, you will probably have rows or cols,
rather than both..
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows
> -Original Message-
> From: ffmpeg-devel On Behalf Of Andreas
> Rheinhardt
> Sent: Saturday, January 15, 2022 10:45 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> Windows
>
> Soft Works:
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of Andreas
> >> Rheinhardt
> >> Sent: Saturday, January 15, 2022 7:34 PM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> >> Windows
> >>
> >> Soft Works:
> >>>
> >>>
> >>>> -Original Message-
> >>>> From: ffmpeg-devel On Behalf Of
> Andreas
> >>>> Rheinhardt
> >>>> Sent: Saturday, January 15, 2022 7:40 AM
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling
> on
> >>>> Windows
> >>>>
> >>>> ffmpegagent:
> >>>>> From: softworkz
> >>>>>
> >>>>> Signed-off-by: softworkz
> >>>>> ---
> >>>>> avformat/hlsenc: Fix path handling on Windows
> >>>>>
> >>>>> Handling for DOS path separators was missing
> >>>>>
> >>>>> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> >>>> ffstaging-19%2Fsoftworkz%2Fsubmit_hlspath-v1
> >>>>> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-
> >>>> 19/softworkz/submit_hlspath-v1
> >>>>> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/19
> >>>>>
> >>>>> libavformat/hlsenc.c | 4
> >>>>> 1 file changed, 4 insertions(+)
> >>>>>
> >>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >>>>> index ef8973cea1..eff7f4212e 100644
> >>>>> --- a/libavformat/hlsenc.c
> >>>>> +++ b/libavformat/hlsenc.c
> >>>>> @@ -3028,6 +3028,10 @@ static int hls_init(AVFormatContext *s)
> >>>>> }
> >>>>>
> >>>>> p = strrchr(vs->m3u8_name, '/');
> >>>>> +#if HAVE_DOS_PATHS
> >>>>> +p = FFMAX(p, strrchr(vs->m3u8_name, '\\'));
> >>>>> +#endif
> >>>>> +
> >>>>> if (p) {
> >>>>> char tmp = *(++p);
> >>>>> *p = '\0';
> >>>>>
> >>>>> base-commit: c936c319bd54f097cc1d75b1ee1c407d53215d71
> >>>>>
> >>>>
> >>>
> >>> Thanks for reviewing.
> >>>
> >>>> 1. You seem to be under the impression that NULL <= all other pointers.
> >>>> This is wrong. Relational operators acting on pointers are only defined
> >>>> when both point to the same object (the case of "one past the last
> >>>> element of an array" is also allowed) and are undefined behaviour
> >> otherwise.
> >>>
> >>> The case about NULL is interesting - I wasn't aware of that.
> >>> Is it practically relevant, i.e. is there any platform where casting
> >>> (void *)0 does not evaluate to 0 ?
> >>>
> >>
> >> "An integer constant expression with the value 0, or such an expression
> >> cast to type
> >> void *, is called a null pointer constant." (C11, 6.3.2.3 3) (void*)0
> >> is therefore a valid null pointer constant and is also commonly used for
> >> the NULL macro. (void*)0 == 0 is always true, because the right hand
> >> side is converted to the type of the pointer (namely to a null pointer)
> >> and all null pointers compare equal. But this is irrelevant to
> >> relational comparisons, because checking for equality of pointers is not
> >> subject to these pointers pointing to the same object (or one past the
> >> last element of an array...), whereas this is so for relational
> operations.
> >>
> >> (If one uses unsigned for pointers, then one only needs to reserve two
> >> values that can not be used as part of an object: 0 and the max value
> >> (the latter can't be used for an object, because using a pointer one
> >> past the object is legal and has to be consistent wi
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows
> -Original Message-
> From: ffmpeg-devel On Behalf Of Soft Works
> Sent: Saturday, January 15, 2022 10:18 PM
> To: FFmpeg development discussions and patches
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> Windows
>
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of Andreas
> > Rheinhardt
> > Sent: Saturday, January 15, 2022 7:34 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> > Windows
> >
> > Soft Works:
> > >
> > >
> > >> -Original Message-
> > >> From: ffmpeg-devel On Behalf Of
> Andreas
> > >> Rheinhardt
> > >> Sent: Saturday, January 15, 2022 7:40 AM
> > >> To: ffmpeg-devel@ffmpeg.org
> > >> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling
> on
> > >> Windows
> > >>
> > >> ffmpegagent:
> > >>> From: softworkz
> > >>>
> > >>> Signed-off-by: softworkz
> > >>> ---
> > >>> avformat/hlsenc: Fix path handling on Windows
> > >>>
> > >>> Handling for DOS path separators was missing
> > >>>
> > >>> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> > >> ffstaging-19%2Fsoftworkz%2Fsubmit_hlspath-v1
> > >>> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-
> > >> 19/softworkz/submit_hlspath-v1
> > >>> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/19
> > >>>
> > >>> libavformat/hlsenc.c | 4
> > >>> 1 file changed, 4 insertions(+)
> > >>>
> > >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > >>> index ef8973cea1..eff7f4212e 100644
> > >>> --- a/libavformat/hlsenc.c
> > >>> +++ b/libavformat/hlsenc.c
> > >>> @@ -3028,6 +3028,10 @@ static int hls_init(AVFormatContext *s)
> > >>> }
> > >>>
> > >>> p = strrchr(vs->m3u8_name, '/');
> > >>> +#if HAVE_DOS_PATHS
> > >>> +p = FFMAX(p, strrchr(vs->m3u8_name, '\\'));
> > >>> +#endif
> > >>> +
> > >>> if (p) {
> > >>> char tmp = *(++p);
> > >>> *p = '\0';
> > >>>
> > >>> base-commit: c936c319bd54f097cc1d75b1ee1c407d53215d71
> > >>>
> > >>
> > >
> > > Thanks for reviewing.
> > >
> > >> 1. You seem to be under the impression that NULL <= all other pointers.
> > >> This is wrong. Relational operators acting on pointers are only defined
> > >> when both point to the same object (the case of "one past the last
> > >> element of an array" is also allowed) and are undefined behaviour
> > otherwise.
> > >
> > > The case about NULL is interesting - I wasn't aware of that.
> > > Is it practically relevant, i.e. is there any platform where casting
> > > (void *)0 does not evaluate to 0 ?
> > >
> >
> > "An integer constant expression with the value 0, or such an expression
> > cast to type
> > void *, is called a null pointer constant." (C11, 6.3.2.3 3) (void*)0
> > is therefore a valid null pointer constant and is also commonly used for
> > the NULL macro. (void*)0 == 0 is always true, because the right hand
> > side is converted to the type of the pointer (namely to a null pointer)
> > and all null pointers compare equal. But this is irrelevant to
> > relational comparisons, because checking for equality of pointers is not
> > subject to these pointers pointing to the same object (or one past the
> > last element of an array...), whereas this is so for relational operations.
> >
> > (If one uses unsigned for pointers, then one only needs to reserve two
> > values that can not be used as part of an object: 0 and the max value
> > (the latter can't be used for an object, because using a pointer one
> > past the object is legal and has to be consistent with "<=" (and anyway
> > said pointer must compare unequal to NULL)); if one used signed
> > comparisons for pointers
>
> On initial read I had thought you were implying something like "signed
> pointers". But maybe one should really create a computer syst
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows
> -Original Message-
> From: ffmpeg-devel On Behalf Of Andreas
> Rheinhardt
> Sent: Saturday, January 15, 2022 7:34 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> Windows
>
> Soft Works:
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of Andreas
> >> Rheinhardt
> >> Sent: Saturday, January 15, 2022 7:40 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> >> Windows
> >>
> >> ffmpegagent:
> >>> From: softworkz
> >>>
> >>> Signed-off-by: softworkz
> >>> ---
> >>> avformat/hlsenc: Fix path handling on Windows
> >>>
> >>> Handling for DOS path separators was missing
> >>>
> >>> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> >> ffstaging-19%2Fsoftworkz%2Fsubmit_hlspath-v1
> >>> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> >> 19/softworkz/submit_hlspath-v1
> >>> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/19
> >>>
> >>> libavformat/hlsenc.c | 4
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >>> index ef8973cea1..eff7f4212e 100644
> >>> --- a/libavformat/hlsenc.c
> >>> +++ b/libavformat/hlsenc.c
> >>> @@ -3028,6 +3028,10 @@ static int hls_init(AVFormatContext *s)
> >>> }
> >>>
> >>> p = strrchr(vs->m3u8_name, '/');
> >>> +#if HAVE_DOS_PATHS
> >>> +p = FFMAX(p, strrchr(vs->m3u8_name, '\\'));
> >>> +#endif
> >>> +
> >>> if (p) {
> >>> char tmp = *(++p);
> >>> *p = '\0';
> >>>
> >>> base-commit: c936c319bd54f097cc1d75b1ee1c407d53215d71
> >>>
> >>
> >
> > Thanks for reviewing.
> >
> >> 1. You seem to be under the impression that NULL <= all other pointers.
> >> This is wrong. Relational operators acting on pointers are only defined
> >> when both point to the same object (the case of "one past the last
> >> element of an array" is also allowed) and are undefined behaviour
> otherwise.
> >
> > The case about NULL is interesting - I wasn't aware of that.
> > Is it practically relevant, i.e. is there any platform where casting
> > (void *)0 does not evaluate to 0 ?
> >
>
> "An integer constant expression with the value 0, or such an expression
> cast to type
> void *, is called a null pointer constant." (C11, 6.3.2.3 3) (void*)0
> is therefore a valid null pointer constant and is also commonly used for
> the NULL macro. (void*)0 == 0 is always true, because the right hand
> side is converted to the type of the pointer (namely to a null pointer)
> and all null pointers compare equal. But this is irrelevant to
> relational comparisons, because checking for equality of pointers is not
> subject to these pointers pointing to the same object (or one past the
> last element of an array...), whereas this is so for relational operations.
>
> (If one uses unsigned for pointers, then one only needs to reserve two
> values that can not be used as part of an object: 0 and the max value
> (the latter can't be used for an object, because using a pointer one
> past the object is legal and has to be consistent with "<=" (and anyway
> said pointer must compare unequal to NULL)); if one used signed
> comparisons for pointers
On initial read I had thought you were implying something like "signed
pointers". But maybe one should really create a computer system with
"symmetric addressing" where pointers are always pointing to the center
of a memory area (odd-number sized). I think it would be a great
invention for bored and under-challenged developers who love to
deep-dive into academic details - hihi..
(I'd bet, concepts and theory exist somewhere anywhere)
> one would have to reserve -1, 0 and the max
> value, the former because a one past the end array element needs to
> compare unequal to NULL and the latter to be consistent with <= and a
> potential one-past-the-end element. But this is a very small advantage.
> Honestly, I don't know whether compilers consistently use unsigned
> comparisons for pointer comparisons at all (even when restric
Re: [FFmpeg-devel] [PATCH 21/24] fftools/ffmpeg: Introduce subtitle filtering new frame-based subtitle encoding
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Friday, January 14, 2022 5:53 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 21/24] fftools/ffmpeg: Introduce subtitle > filtering new frame-based subtitle encoding > > On Fri, Jan 14, 2022 at 01:13:30AM +, ffmpegagent wrote: > > From: softworkz > > > > This commit actually enables subtitle filtering in ffmpeg by > > sending and receiving subtitle frames to and from a filtergraph. > > > > The heartbeat functionality from the previous sub2video implementation > > is removed and now provided by the 'subfeed' filter. > > The other part of sub2video functionality is retained by > > auto-insertion of the new graphicsub2video filter. > > > > Justification for changed test refs: > > > > - sub2video > > The new results are identical excepting the last frame which > > is due to the implementation changes > > > > - sub2video_basic > > The previous results had some incorrect output because multiple > > frames had the same dts > > The non-empty content frames are visually identical, the different > > CRC is due to the different blending algorithm that is being used. > > > > - sub2video_time_limited > > The third frame in the previous ref was a repetition, which doesn't > > happen anymore with the new subtitle filtering. > > > > - sub-dvb > > Running ffprobe -show_frames on the source file shows that there > > are 7 subtitle frames with 0 rects in the source at the start > > and 2 at the end. This translates to the 14 and 4 additional > > entries in the new test results. > > > > - filter-overlay-dvdsub-2397 > > Overlay results have slightly different CRCs due to different > > blending implementation > > > > Signed-off-by: softworkz > > --- > > fftools/ffmpeg.c | 493 > > fftools/ffmpeg.h | 13 +- > > fftools/ffmpeg_filter.c | 235 ++-- > > fftools/ffmpeg_hw.c |2 +- > > fftools/ffmpeg_opt.c |3 +- > > tests/ref/fate/filter-overlay-dvdsub-2397 | 182 +-- > > tests/ref/fate/sub-dvb| 162 +-- > > tests/ref/fate/sub2video | 1091 +- > > tests/ref/fate/sub2video_basic| 1239 +++-- > > tests/ref/fate/sub2video_time_limited | 78 +- > > 10 files changed, 2829 insertions(+), 669 deletions(-) > > ./ffmpeg -i ~/tickets/153/bbc_small.ts -filter_complex '[0:v][0:s]overlay' - > qscale 2 -t 3 f.avi > > Press [q] to stop, [?] for help > Auto-inserting subfeed filter > Auto-inserting graphicsub2video filter > Assertion c > 0 failed at libavutil/mathematics.c:60 > Aborted (core dumped) Thanks for testing. With the new update also incorporating the final step (complete removal of heartbeat code), it is unfortunately invalidating the previous test results. The new version is providing the subfeed filter as a replacement for the heartbeat functionality, which is working somewhat differently, and even more tests (of those you are running) won't have bitexact results. In those cases a visual comparison will be required. Anyway, the previous results were erroneous in several ways, so it cannot be seen as a straight reference. The individual results need to be evaluated individually in case there are significant differences. Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 06/24] avcodec/subtitles: Migrate subtitle encoders to frame-based API
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Friday, January 14, 2022 6:22 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 06/24] avcodec/subtitles: Migrate subtitle > encoders to frame-based API > > On Fri, Jan 14, 2022 at 01:13:15AM +, ffmpegagent wrote: > > From: softworkz > > > > and provide a compatibility shim for the legacy api > > > > Signed-off-by: softworkz > > --- > > libavcodec/assenc.c| 189 ++--- > > libavcodec/avcodec.h | 5 +- > > libavcodec/dvbsubenc.c | 96 ++- > > libavcodec/dvdsubenc.c | 102 > > libavcodec/encode.c| 57 ++- > > libavcodec/movtextenc.c| 114 -- > > libavcodec/srtenc.c| 108 ++--- > > libavcodec/tests/avcodec.c | 2 - > > libavcodec/ttmlenc.c | 101 +++- > > libavcodec/webvttenc.c | 86 - > > libavcodec/xsubenc.c | 88 ++--- > > 11 files changed, 685 insertions(+), 263 deletions(-) > > is this supposed to pass make fate without the later patches ? Yea, I've seen it on Patchwork. I forgot to check the individual commits. They need some reordering. Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 03/24] avcodec/subtitles: Introduce new frame-based subtitle decoding API
> -Original Message-
> From: ffmpeg-devel On Behalf Of Andreas
> Rheinhardt
> Sent: Friday, January 14, 2022 6:54 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 03/24] avcodec/subtitles: Introduce new
> frame-based subtitle decoding API
>
> ffmpegagent:
> > From: softworkz
> >
> > - Add avcodec_decode_subtitle3 which takes subtitle frames,
> > serving as compatibility shim to legacy subtitle decoding
> > - Add additional methods for conversion between old and new API
>
> This commit message is completely wrong.
>
> >
> > Signed-off-by: softworkz
> > ---
> > libavcodec/avcodec.h| 8 +-
> > libavcodec/codec_desc.c | 11 +++
> > libavcodec/codec_desc.h | 8 ++
> > libavcodec/decode.c | 56 ++--
> > libavcodec/internal.h | 22 +
> > libavcodec/utils.c | 184
> > 6 files changed, 280 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index fe5a83cf85..9d59f6e840 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -1675,7 +1675,7 @@ typedef struct AVCodecContext {
> >
> > /**
> > * Header containing style information for text subtitles.
> > - * For SUBTITLE_ASS subtitle type, it should contain the whole ASS
> > + * For AV_SUBTITLE_FMT_ASS subtitle type, it should contain the whole
> ASS
> > * [Script Info] and [V4+ Styles] section, plus the [Events] line and
> > * the Format line following. It shouldn't include any Dialogue line.
> > * - encoding: Set/allocated/freed by user (before avcodec_open2())
> > @@ -2415,7 +2415,10 @@ int avcodec_close(AVCodecContext *avctx);
> > * Free all allocated data in the given subtitle struct.
> > *
> > * @param sub AVSubtitle to free.
> > + *
> > + * @deprecated Use the regular frame based encode and decode APIs instead.
> > */
> > +attribute_deprecated
> > void avsubtitle_free(AVSubtitle *sub);
> >
> > /**
> > @@ -2508,7 +2511,10 @@ enum AVChromaLocation avcodec_chroma_pos_to_enum(int
> xpos, int ypos);
> > * must be freed with avsubtitle_free if *got_sub_ptr is
> set.
> > * @param[in,out] got_sub_ptr Zero if no subtitle could be decompressed,
> otherwise, it is nonzero.
> > * @param[in] avpkt The input AVPacket containing the input buffer.
> > + *
> > + * @deprecated Use the new decode API (avcodec_send_packet,
> avcodec_receive_frame) instead.
> > */
> > +attribute_deprecated
> > int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
> > int *got_sub_ptr,
> > AVPacket *avpkt);
> > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> > index 0974ee03de..e48e4532ba 100644
> > --- a/libavcodec/codec_desc.c
> > +++ b/libavcodec/codec_desc.c
> > @@ -3548,3 +3548,14 @@ enum AVMediaType avcodec_get_type(enum AVCodecID
> codec_id)
> > const AVCodecDescriptor *desc = avcodec_descriptor_get(codec_id);
> > return desc ? desc->type : AVMEDIA_TYPE_UNKNOWN;
> > }
> > +
> > +enum AVSubtitleType avcodec_descriptor_get_subtitle_format(const
> AVCodecDescriptor *codec_descriptor)
> > +{
> > +if(codec_descriptor->props & AV_CODEC_PROP_BITMAP_SUB)
> > +return AV_SUBTITLE_FMT_BITMAP;
> > +
> > +if(codec_descriptor->props & AV_CODEC_PROP_TEXT_SUB)
> > +return AV_SUBTITLE_FMT_ASS;
> > +
> > +return AV_SUBTITLE_FMT_UNKNOWN;
> > +}
> > diff --git a/libavcodec/codec_desc.h b/libavcodec/codec_desc.h
> > index 126b52df47..ba68d24e0e 100644
> > --- a/libavcodec/codec_desc.h
> > +++ b/libavcodec/codec_desc.h
> > @@ -121,6 +121,14 @@ const AVCodecDescriptor *avcodec_descriptor_next(const
> AVCodecDescriptor *prev);
> > */
> > const AVCodecDescriptor *avcodec_descriptor_get_by_name(const char *name);
> >
> > +/**
> > + * Return subtitle format from a codec descriptor
> > + *
> > + * @param codec_descriptor codec descriptor
> > + * @return the subtitle type (e.g. bitmap, text)
> > + */
> > +enum AVSubtitleType avcodec_descriptor_get_subtitle_format(const
> AVCodecDescriptor *codec_descriptor);
> > +
> > /**
> > * @}
> > */
> > diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> > index 0912f86a14..ab8a6ea6ff 100644
> > --- a/libavcodec/decode.c
> > +++ b/libavcodec/decode.c
> > @@ -576,6 +576,39 @@ static int
> decode_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> > return ret;
> > }
> >
> > +static int decode_subtitle2_priv(AVCodecContext *avctx, AVSubtitle *sub,
> > + int *got_sub_ptr, AVPacket *avpkt);
> > +
> > +static int decode_subtitle_shim(AVCodecContext *avctx, AVFrame *frame,
> AVPacket *avpkt)
> > +{
> > +int ret, got_sub_ptr = 0;
> > +AVSubtitle subtitle = { 0 };
> > +
> > +if (frame->buf[0])
> > +return AVERROR(EAGAIN);
> > +
> > +av_frame_unref(frame);
> > +
> > +ret = decode_subtitle2_priv(avctx, ,
Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on Windows
> -Original Message-
> From: ffmpeg-devel On Behalf Of Andreas
> Rheinhardt
> Sent: Saturday, January 15, 2022 7:40 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: Fix path handling on
> Windows
>
> ffmpegagent:
> > From: softworkz
> >
> > Signed-off-by: softworkz
> > ---
> > avformat/hlsenc: Fix path handling on Windows
> >
> > Handling for DOS path separators was missing
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-19%2Fsoftworkz%2Fsubmit_hlspath-v1
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> 19/softworkz/submit_hlspath-v1
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/19
> >
> > libavformat/hlsenc.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index ef8973cea1..eff7f4212e 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -3028,6 +3028,10 @@ static int hls_init(AVFormatContext *s)
> > }
> >
> > p = strrchr(vs->m3u8_name, '/');
> > +#if HAVE_DOS_PATHS
> > +p = FFMAX(p, strrchr(vs->m3u8_name, '\\'));
> > +#endif
> > +
> > if (p) {
> > char tmp = *(++p);
> > *p = '\0';
> >
> > base-commit: c936c319bd54f097cc1d75b1ee1c407d53215d71
> >
>
Thanks for reviewing.
> 1. You seem to be under the impression that NULL <= all other pointers.
> This is wrong. Relational operators acting on pointers are only defined
> when both point to the same object (the case of "one past the last
> element of an array" is also allowed) and are undefined behaviour otherwise.
The case about NULL is interesting - I wasn't aware of that.
Is it practically relevant, i.e. is there any platform where casting
(void *)0 does not evaluate to 0 ?
> 2. Apart from that: Your code would potentially evaluate strrchr()
> multiple times which is bad style (given that this function is likely
> marked as pure the compiler could probably optimize the second call
> away, but this is not a given).
It's not my code. It's code copied from avstring.c - so please blame
whoever that wrote.
Regarding performance, I'm not sure whether this is relevant in any way,
given the low frequency of execution and putting it into relation to
all the other things that ffmpeg is doing usually.
> 3. The code in av_basename() is also wrong.
...
> 4. Is there actually a reason why you don't use av_basename() directly here?
Yes, multiple:
1. Different behavior - those functions are returning a "." when not found
2. The docs tell it's required to copy a string before supplying it to
those functions (as they may changing the string).
3. The hlsenc code changes the string temporarily and restores it after
wards. The same couldn't be done when using the avstring functions.
Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] 5.0 blocking issues
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Wednesday, January 12, 2022 5:31 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] 5.0 blocking issues > > On Wed, Jan 12, 2022 at 07:37:03AM +0100, Lynne wrote: > > 8 Jan 2022, 17:30 by mich...@niedermayer.cc: > > > > > Hi all > > > > > > This is a simple go/no go call > > > if you know of something that still should go into 5.0 please reply here > > > with a list of what you are working on and a timelimit until when you > > > will be done with it > > > > > > if you think everything is ready for the release, then too feel free to > > > reply (assuming few others said ok yet) this is not supposed to become a > > > 100 reply thread with oks, just maybe 2-3 people confirming that noone > > > is aware of things missing. > > > > > > I intend to do the release within 2-3 days of all "no go" things being > > > resolved or timeouting > > > > > > thx > > > > > > > Both of elenril and mkver's patches got merged, isn't it > > time to tag and make archives already? > > something is not working efficiently here > There have been more replies here than from elenril and mkver > also there have been over the ML multiple requests for backports > to 5.0 today. (which may or may not be all unimportant) > We do need a better way to detect when a branch is ready for release > or master is ready to be branched. > Iam really bad at keeping track of everything everyone asks to be > included and then notice when it was included fully with nothing > missing and no amendmends. > Maybe we could put a RELEASE_BLOCKER file in release branches in the future > and everyone can list in it what needs to be done before the release > and when something is done the person pushing would also remove that > line from the file. > > for 5.0 now, i guess this is a 2nd go/no go call here > anything else that I should wait for ? > if not i will make the release _probably_ in the next 2-3 days or so > (if nothing interferes) [PATCH v4] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11) Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-15%2Fsoftworkz%2Fqsv_vpp_regression-v4 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15 Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.15.v4.ffstaging.ffmpeg.1641538891098.ffmpegag...@gmail.com/ => Has been merged in to master [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-16%2Fsoftworkz%2Fpatch_dvdsubdec_fix-v1 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/16 Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.16.ffstaging.ffmpeg.1641262759164.ffmpegag...@gmail.com/ => not merged yet Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Monday, January 10, 2022 9:57 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 10/01/2022 01:40, Soft Works wrote: > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Mark > >> Thompson > >> Sent: Monday, January 10, 2022 1:57 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When > deriving a > >> hwdevice, search for existing device in both directions > >> > >> [trimmed somewhat] > > To wrap things up a bit: > > > > - you want an approach which requires even more complicated filter > >command lines. > > Ha, that characterisation isn't exactly neutral - the derivation in filter > graphs is removed and replaced with explicit specification of devices. The comparison in your other reply wasn't realistic. Actually it would look similar to this: BEFORE ffmpeg -hwaccel qsv -c:v h264_qsv -i input.mp4 -vf "hwmap=derive_device=vaapi, format=vaapi,hwmap=derive_device=vulkan,scale_vulkan=w=1920:h=1080" -f null - AFTER ffmpeg -init_hw_device vaapi=vadev -init_hw_device qsv=qsvdev@vadev -init_hw_device vulkan=vkdev@vadev -hwaccel qsv -c:v h264_qsv -i input.mp4 -vf "hwmap=hw_device=vadev,format=vaapi,hwmap=hw_device=vkdev,scale_vulkan=w= 1920:h=1080" -f null - This is more like a ** to all those who are writing command lines by hand and surely not an improvement. I have not been involved in the cli design, but from looking at the result it's clear that there has always been a focus on allowing things to be done as easily as possible instead of needing to do extensive reading until you get it right (or give up). > Perhaps you even want to add to the av_hwdevice_ctx_create_derived() API so > it can take a YKNOW_ACTUALLY_DONT flag to indicate you don't want to create a > new derived device, I don't know. When you mean to add av_hwdevice_ctx_create_derived2() or av_hwdevice_ctx_create_derived_ex() which takes an additional parameter to control this and which the ffmpeg-tool will use instead, that would be fine I think. It would allow to make the change only for the cl tool without affecting API consumers (regardless of whether such cases may exist or not). softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v1] avfilter/tonemap_vaapi: set va parameters filters and numbers
> -Original Message-
> From: ffmpeg-devel On Behalf Of Xiang,
> Haihao
> Sent: Wednesday, January 12, 2022 5:16 AM
> To: FFmpeg development discussions and patches
> Cc: Wang, Fei W
> Subject: Re: [FFmpeg-devel] [PATCH v1] avfilter/tonemap_vaapi: set va
> parameters filters and numbers
>
>
> > -Original Message-
> > From: ffmpeg-devel On Behalf Of Fei
> > Wang
> > Sent: Tuesday, June 30, 2020 09:19
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Wang, Fei W
> > Subject: [FFmpeg-devel] [PATCH v1] avfilter/tonemap_vaapi: set va
> parameters
> > filters and numbers
> >
> > This can fill VAProcPipelineParameterBuffer correctly and make the
> > pipeline works.
> >
> > Signed-off-by: Fei Wang
> > ---
> > libavfilter/vf_tonemap_vaapi.c | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/libavfilter/vf_tonemap_vaapi.c
> b/libavfilter/vf_tonemap_vaapi.c
> > index 2f41b90424..9f52590a49 100644
> > --- a/libavfilter/vf_tonemap_vaapi.c
> > +++ b/libavfilter/vf_tonemap_vaapi.c
> > @@ -296,6 +296,11 @@ static int tonemap_vaapi_filter_frame(AVFilterLink
> > *inlink, AVFrame *input_frame
> > if (err < 0)
> > goto fail;
> >
> > +if (vpp_ctx->nb_filter_buffers) {
> > +params.filters = _ctx->filter_buffers[0];
> > +params.num_filters = vpp_ctx->nb_filter_buffers;
> > +}
> > +
> > err = ff_vaapi_vpp_render_picture(avctx, , output_frame);
> > if (err < 0)
> > goto fail;
>
> This fixed the broken tonemap_vaapi filter, I'll apply it if no objection.
LGTM. I have a similar patch.
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] QSV: Introduce min Compile-SDK Version and check for Runtime-Versions instead
> -Original Message- > From: ffmpeg-devel On Behalf Of Xiang, > Haihao > Sent: Wednesday, January 12, 2022 3:53 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [RFC] QSV: Introduce min Compile-SDK Version and > check for Runtime-Versions instead > > On Mon, 2022-01-03 at 21:53 +, Soft Works wrote: > > Hi, > > > > this is a follow-up to my recently submitted patch: > > > > “avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)” > > > > That patch only fixes the one important regression from multiple issues > which > > have been introduced by recent changes. Those changes have gone in a less > > than ideal direction, because they have introduced checks based on the > version > > of the MSDK which ffmpeg is being compiled against, but they didn’t add > > checks for the runtime libmfx/MSDK versions – and this causes failures, > > sometimes even for functionality which has worked before (=>regressions). > > > > Background > > > > - the compile-sdk version determines which features can be used > > but only when > > - the runtime SDK version supports it > > and > > - the hardware (GPU gen) supports it > > - ALL compile-sdk versions can interface with ALL runtime MSDK > > versions (basically), no matter whether runtime version is newer or > > older > > - At least on Windows, some hw is stuck at a certain runtime version, > > e.g.: there are new drivers for Broadwell, but the MSDK runtime > > is always 1.11 > > > > > > Conclusion > > > > Adding checks for the runtime MSDK versions is required wherever a feature > > might not be supported by older MSDK runtimes - I think that's an obvious > > necessity. > > > > > > Question > > > > Having both - run-time and compile-time checks all over the code is adding > > a lot of complexity and makes it difficult to maintain and work with. > > > > Hence, I'm wondering whether we couldn't/shouldn't introduce a minimum > > MSDK compile-time version, for example 1.22, or even later? > > > > This would allow simplification of the QSV code in many places where run- > time > > version checks are actually needed instead. > > > > Over time, there have been better and worse MSDK versions, and there > > should still be enough room for choosing, but I don't think there's any > > reason why somebody would still want to compile against some really old > > (e.g. < 1.22) MSDK version. > > > > Please share your thoughts on this subject.. > > I agree we may add a requirement for the minimal compiling version in > configure, > version 1.28 was released 3 years ago, how about using this version as the > minimal compiling version ? I'm fine with 1.28! > BTW you may download the new Windows drivers for Broadwell from > https://www.intel.com/content/www/us/en/download/18369/intel-graphics-driver- > for-windows-15-40.html > , the runtime version should be 1.20 My mistake, I mixed the code names up. It was about a 3rd gen CPU. Those are stuck at 1.11. 4th and 5th gen are stuck at 1.20 and the others get the latest MSDK runtime. It's like I had described in 2020 here: https://github.com/softworkz/ffmpeg_dx11/issues/1 Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
> -Original Message- > From: ffmpeg-devel On Behalf Of Hendrik > Leppkes > Sent: Monday, January 10, 2022 11:31 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't > dump images to disk based on DEBUG define > > On Fri, Jan 7, 2022 at 5:14 PM Soft Works wrote: > > > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of Hendrik > > > Leppkes > > > Sent: Friday, January 7, 2022 11:32 AM > > > To: FFmpeg development discussions and patches > > > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: > don't > > > dump images to disk based on DEBUG define > > > > > > On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes > wrote: > > > > > > > > On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent > wrote: > > > > > > > > > > It's annoying and unexpected, but still useful at times (as I've > realized > > > > > just recently). > > > > > > > > > > This is a follow-up to the earlier submission here: > > > > > https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html > > > > > > > > > > There has been a comment from Anton, questioning whether the dump- > feature > > > is > > > > > useful. Meanwhile I came to the conclusion that it can be useful in- > fact. > > > It > > > > > just shouldn't happen automatically when DEBUG is defined. That's > what > > > these > > > > > patches do. > > > > > > > > > > I also added fixes for the fopen() call. > > > > > > > > > > softworkz (4): > > > > > avcodec/dvbsubdec: don't dump images to disk based on DEBUG define > > > > > avcodec/dvbsubdec: fix writing ppm > > > > > avcodec/dvdsubdec: don't dump images to disk based on DEBUG define > > > > > avcodec/dvdsubdec: fix writing ppm > > > > > > > > > > libavcodec/dvbsubdec.c | 20 +--- > > > > > libavcodec/dvdsubdec.c | 11 --- > > > > > 2 files changed, 21 insertions(+), 10 deletions(-) > > > > > > > > > > > > > > > > > > These patches need some squasing. It should've been obvious from them > > > > being duplicated. > > > > > > > > > > Actually I fell into the dvb dvd trap, but regardless the patches are > > > basically identical, so I would still squash them, personally. > > > > No problem, but in which way? > > > > 1. All 4 commits into a single one > > 2. 4 >> 2 horizontally (a single one for the debug commits and a single > >one for the fopen commits) > > 3. 4 >> 2 vertically (a single commit for dvb and a single one for dvd) > > > > I figured combining the patches that do the exact same thing for the > two different decoders. OK, will do. Thank you, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> -Original Message-
> From: Xiang, Haihao
> Sent: Monday, January 10, 2022 8:03 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: haihao.xiang-at-intel@ffmpeg.org; softwo...@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v4] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
>
> On Fri, 2022-01-07 at 07:01 +, ffmpegagent wrote:
> > From: softworkz
> >
> > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way that scaling via vpp_qsv doesn't work any longer
> > for devices with an MSDK runtime version lower than 1.19. This is true
> > for older CPUs which are stuck at 1.11.
> > The commit added checks for the compile-sdk version but it didn't test
> > for the runtime version.
> >
> > Signed-off-by: softworkz
> > ---
> > avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> >
> > Fix a recently introduced regression when using QSV VPP.
> >
> > v2: Fixed commit message wrapping
> >
> > v3: Use different way to acquire runtime version
> >
> > v4: Use mfxStatus as return type for get_mfx_version()
> >
> > Published-As:
> > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 15%2Fsoftworkz%2Fqsv_vpp_regression-v4
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > 15/softworkz/qsv_vpp_regression-v4
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> >
> > Range-diff vs v3:
> >
> > 1: cbf53d8ef4 ! 1: bf53154420 avfilter/vpp_qsv: fix regression on older
> api
> > versions (e.g. 1.11)
> > @@ libavfilter/vf_vpp_qsv.c: static int config_input(AVFilterLink
> > *inlink)
> >return 0;
> >}
> >
> > -+static int get_mfx_version(const AVFilterContext *ctx, mfxVersion
> > *mfx_version)
> > ++static mfxStatus get_mfx_version(const AVFilterContext *ctx,
> mfxVersion
> > *mfx_version)
> > +{
> > +const AVFilterLink *inlink = ctx->inputs[0];
> > +AVBufferRef *device_ref;
> > @@ libavfilter/vf_vpp_qsv.c: static int config_input(AVFilterLink
> > *inlink)
> > +// so don't error here but let runtime version checks fail
> by
> > setting to 0.0
> > +mfx_version->Major = 0;
> > +mfx_version->Minor = 0;
> > -+return 0;
> > ++return MFX_ERR_NONE;
> > +}
> > +
> > +device_ctx = (AVHWDeviceContext *)device_ref->data;
> > @@ libavfilter/vf_vpp_qsv.c: static int config_output(AVFilterLink
> > *outlink)
> >param.ext_buf = ext_buf;
> >param.async_depth = vpp->async_depth;
> >
> > -+if (get_mfx_version(ctx, _version) < 0) {
> > ++if (get_mfx_version(ctx, _version) != MFX_ERR_NONE) {
> > +av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> > +return AVERROR(EINVAL);
> > +}
> >
> >
> > libavfilter/vf_vpp_qsv.c | 59
> > 1 file changed, 47 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index ab58a5777e..5c96703fd3 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -26,6 +26,7 @@
> > #include "libavutil/opt.h"
> > #include "libavutil/eval.h"
> > #include "libavutil/hwcontext.h"
> > +#include "libavutil/hwcontext_qsv.h"
> > #include "libavutil/pixdesc.h"
> > #include "libavutil/mathematics.h"
> >
> > @@ -139,8 +140,9 @@ static const AVOption options[] = {
> > { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> > { "format", "Output pixel format", OFFSET(output_format_str),
> > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > { "async_depth", "Internal parallelization depth, the higher the value
> > the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0
> },
> > 0, INT_MAX, .flags = FLAGS },
> > +#ifdef QSV_HAVE_SCALING_CONFIG
> > { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> > OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> > mode" },
> > -
> > +#endif
> > { NULL }
> > };
> >
> > @@ -297,6 +299,32 @@ static int config_input(AVFilterLink *inlink)
> > return 0;
> > }
> >
> > +static mfxStatus get_mfx_version(const AVFilterContext *ctx, mfxVersion
> > *mfx_version)
> > +{
> > +const AVFilterLink *inlink = ctx->inputs[0];
> > +AVBufferRef *device_ref;
> > +AVHWDeviceContext *device_ctx;
> > +AVQSVDeviceContext *device_hwctx;
> > +
> > +if (inlink->hw_frames_ctx) {
> > +AVHWFramesContext *frames_ctx = (AVHWFramesContext *)inlink-
> > >hw_frames_ctx->data;
> > +device_ref = frames_ctx->device_ref;
> > +} else if (ctx->hw_device_ctx) {
> > +device_ref = ctx->hw_device_ctx;
> > +} else {
> > +
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Monday, January 10, 2022 1:57 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 09/01/2022 23:36, Soft Works wrote:>> -Original Message- > >> From: ffmpeg-devel On Behalf Of Mark > >> Thompson > >> Sent: Monday, January 10, 2022 12:13 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When > deriving a > >> hwdevice, search for existing device in both directions > >> > >> On 09/01/2022 21:15, Soft Works wrote: > >>>> -Original Message- > >>>> From: ffmpeg-devel On Behalf Of Mark > >>>> Thompson > >>>> Sent: Sunday, January 9, 2022 7:39 PM > >>>> To: ffmpeg-devel@ffmpeg.org > >>>> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When > >> deriving a > >>>> hwdevice, search for existing device in both directions > >>>> > >>>> On 05/01/2022 03:38, Xiang, Haihao wrote: > >>>>> ... this patch really fixed some issues for me and others. > >>>> > >>>> Can you explain this in more detail? > >>>> > >>>> I'd like to understand whether the issues you refer to are something > which > >>>> would be fixed by the ffmpeg utility allowing selection of devices for > >>>> libavfilter, or whether they are something unrelated. > >>>> > >>>> (For library users the currently-supported way of getting the same > device > >>>> again is to keep a reference to the device and reuse it. If there is > some > >>>> case where you can't do that then it would be useful to hear about it.) > >>> > >>> Hi Mark, > >>> > >>> they have 3 workaround patches on their staging repo, but I'll let Haihao > >>> answer in detail. > >>> > >>> I have another question. I've been searching high and low, yet I can't > >>> find the message. Do you remember that patch discussion from (quite a > >>> few) months ago, where it was about another QSV change (something about > >>> device creation from the command line, IIRC). There was a command line > >>> example with QSV and you correctly remarked something like: > >>> "Do you even know that just for this command line, there are 5 device > >>> creations happening in the background, implicit and explicit, and in > >>> one case (or 2), it's not even creating the specified device but > >>> a session for the default device instead" > >>> (just roughly from memory) > >>> > >>> Do you remember - or was it Philip? > >> > >> <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277731.html> > >> > >>> Anyway, this is something that the patch will improve. There has been one > >>> other commit since that time regarding explicit device creation from > >>> Haihao (iirc), which already reduced the device creation and fixed the > >>> incorrect default session creation. > >> > >> Yes, the special ffmpeg utility code to work around the lack of > >> AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX in the libmfx decoders caused > >> confusion by working differently to everything else - implementing that > and > >> getting rid of the workarounds was definitely a good thing. > >> > >>> My patch tackles this from another side: at that time, you (or Philip) > >>> explained that the secondary context that QSV requires (VAAPI, D3Dx) > >>> and that is initially created when setting up the QSV device, does not > >>> even get used when subsequently deriving to a context of that kind. > >>> Instead, a new device is being created in this case. > >>> > >>> That's another scenario which is fixed by this patch. > >> > >> It does sound like you just always want a libmfx device to be derived from > >> the thing which is really there sitting underneath it. > > > > "That's another scenario which is fixed by this patch" > > > > Things stop working as expected as soon as you are working with 3 or more > > derived hw devices and neither hwmap nor hwmap-revere can get you to the > > context you want. > > And your
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Monday, January 10, 2022 12:13 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 09/01/2022 21:15, Soft Works wrote: > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Mark > >> Thompson > >> Sent: Sunday, January 9, 2022 7:39 PM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When > deriving a > >> hwdevice, search for existing device in both directions > >> > >> On 05/01/2022 03:38, Xiang, Haihao wrote: > >>> ... this patch really fixed some issues for me and others. > >> > >> Can you explain this in more detail? > >> > >> I'd like to understand whether the issues you refer to are something which > >> would be fixed by the ffmpeg utility allowing selection of devices for > >> libavfilter, or whether they are something unrelated. > >> > >> (For library users the currently-supported way of getting the same device > >> again is to keep a reference to the device and reuse it. If there is some > >> case where you can't do that then it would be useful to hear about it.) > > > > Hi Mark, > > > > they have 3 workaround patches on their staging repo, but I'll let Haihao > > answer in detail. > > > > I have another question. I've been searching high and low, yet I can't > > find the message. Do you remember that patch discussion from (quite a > > few) months ago, where it was about another QSV change (something about > > device creation from the command line, IIRC). There was a command line > > example with QSV and you correctly remarked something like: > > "Do you even know that just for this command line, there are 5 device > > creations happening in the background, implicit and explicit, and in > > one case (or 2), it's not even creating the specified device but > > a session for the default device instead" > > (just roughly from memory) > > > > Do you remember - or was it Philip? > > <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2021-March/277731.html> > > > Anyway, this is something that the patch will improve. There has been one > > other commit since that time regarding explicit device creation from > > Haihao (iirc), which already reduced the device creation and fixed the > > incorrect default session creation. > > Yes, the special ffmpeg utility code to work around the lack of > AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX in the libmfx decoders caused > confusion by working differently to everything else - implementing that and > getting rid of the workarounds was definitely a good thing. > > > My patch tackles this from another side: at that time, you (or Philip) > > explained that the secondary context that QSV requires (VAAPI, D3Dx) > > and that is initially created when setting up the QSV device, does not > > even get used when subsequently deriving to a context of that kind. > > Instead, a new device is being created in this case. > > > > That's another scenario which is fixed by this patch. > > It does sound like you just always want a libmfx device to be derived from > the thing which is really there sitting underneath it. "That's another scenario which is fixed by this patch" Things stop working as expected as soon as you are working with 3 or more derived hw devices and neither hwmap nor hwmap-revere can get you to the context you want. > If you are a library user then you get the original hw context by reusing the > reference to it that you made when you created it. This includes libavfilter > users, who can provide a hw device to each hwmap separately. > > If you are an ffmpeg utility user then I agree there isn't currently a way to > do this for filter graphs, hence the solution of providing an a way in the > ffmpeg utility to set hw devices per-filter. just setting the context on a filter doesn't make any sense, because you need the mapping. It only makes sense for the hwmap and hwupload filters. > > Anyway I'm wondering whether it can even be logically valid to derive > > from one device to another and then to another instance of the previous > > device type. > > From my understanding, "deriving" or "hw mapping" from one device to > > another means to establish a different way or accessor to a common > > resource/data, which means that you can access the data in one o
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Sunday, January 9, 2022 7:39 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 05/01/2022 03:38, Xiang, Haihao wrote: > > ... this patch really fixed some issues for me and others. > > Can you explain this in more detail? > > I'd like to understand whether the issues you refer to are something which > would be fixed by the ffmpeg utility allowing selection of devices for > libavfilter, or whether they are something unrelated. > > (For library users the currently-supported way of getting the same device > again is to keep a reference to the device and reuse it. If there is some > case where you can't do that then it would be useful to hear about it.) Hi Mark, they have 3 workaround patches on their staging repo, but I'll let Haihao answer in detail. I have another question. I've been searching high and low, yet I can't find the message. Do you remember that patch discussion from (quite a few) months ago, where it was about another QSV change (something about device creation from the command line, IIRC). There was a command line example with QSV and you correctly remarked something like: "Do you even know that just for this command line, there are 5 device creations happening in the background, implicit and explicit, and in one case (or 2), it's not even creating the specified device but a session for the default device instead" (just roughly from memory) Do you remember - or was it Philip? Anyway, this is something that the patch will improve. There has been one other commit since that time regarding explicit device creation from Haihao (iirc), which already reduced the device creation and fixed the incorrect default session creation. My patch tackles this from another side: at that time, you (or Philip) explained that the secondary context that QSV requires (VAAPI, D3Dx) and that is initially created when setting up the QSV device, does not even get used when subsequently deriving to a context of that kind. Instead, a new device is being created in this case. That's another scenario which is fixed by this patch. It's a hybrid device context, that's the reason why QSV is more affected than all other hwaccels as it consists of a QSV session already DERIVED from a VAAPI or D3Dx device. Example (let's assume Windows with D3D9): You go into decoding with a QSV decoder in a QSV context and then you want to use an OpenCL filter. This requires an OpenCL context, and of course you want to share the frames memory. For memory sharing, OpenCL requires the underlying context of the QSV session - in this example D3D9. Before this patch - like you said - deriving devices was usually (except reverse hwmap) forward-only. That means - you are stuck in this situation: you could (forward-)derive to a D3D9 context, but that doesn't help: for sharing the memory, you need to provide the original hw device to OpenCL, you can't supply just another newly derived device of the same type. And there is (was) no way to get the original hw context. Anyway I'm wondering whether it can even be logically valid to derive from one device to another and then to another instance of the previous device type. >From my understanding, "deriving" or "hw mapping" from one device to another means to establish a different way or accessor to a common resource/data, which means that you can access the data in one or the other way. Now let's assume a forward device-derivation chain like this: D3D_1 >> OpenCL_1 >> D3D_2 We have D3D surfaces, then we share them with OpenCL. Both *_1 contexts provide access to the same data. Then we derive again "forward only" and we get a new D3D_2 context. It is derived from OpenCL_1, so it must provide access to the same data as OpenCL_1 AND D3D_1. Now we have two different D3D contexts which are supposed to provide access to the same data! 1. This doesn't even work technically - neither from D3D (iirc) - nor from ffmpeg (not cleanly) 2. This doesn't make sense at all. There should always be only a single device context of a device type for the same resource 3. Why would somebody even want this - what kind of use case? Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] 5.0 blocking issues
> -Original Message- > From: ffmpeg-devel On Behalf Of Michael > Niedermayer > Sent: Saturday, January 8, 2022 5:30 PM > To: FFmpeg development discussions and patches > Subject: [FFmpeg-devel] 5.0 blocking issues > > Hi all > > This is a simple go/no go call > if you know of something that still should go into 5.0 please reply here > with a list of what you are working on and a timelimit until when you > will be done with it [PATCH v4] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11) Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-15%2Fsoftworkz%2Fqsv_vpp_regression-v4 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15 Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.15.v4.ffstaging.ffmpeg.1641538891098.ffmpegag...@gmail.com/ [PATCH] avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-16%2Fsoftworkz%2Fpatch_dvdsubdec_fix-v1 Pull-Request: https://github.com/ffstaging/FFmpeg/pull/16 Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/patch/pull.16.ffstaging.ffmpeg.1641262759164.ffmpegag...@gmail.com/ Both are completed. Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
> -Original Message- > From: ffmpeg-devel On Behalf Of Marton > Balint > Sent: Friday, January 7, 2022 10:53 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't > dump images to disk based on DEBUG define > > > > On Fri, 7 Jan 2022, Soft Works wrote: > > > > > > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Marton > >> Balint > >> Sent: Friday, January 7, 2022 8:57 PM > >> To: FFmpeg development discussions and patches > >> Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: > don't > >> dump images to disk based on DEBUG define > >> > >> > >> > >> On Fri, 7 Jan 2022, ffmpegagent wrote: > >> > >>> It's annoying and unexpected, but still useful at times (as I've realized > >>> just recently). > >>> > >>> This is a follow-up to the earlier submission here: > >>> https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html > >>> > >>> There has been a comment from Anton, questioning whether the dump-feature > >> is > >>> useful. Meanwhile I came to the conclusion that it can be useful in-fact. > >> It > >>> just shouldn't happen automatically when DEBUG is defined. That's what > >> these > >>> patches do. > >> > >> Well, I kind of agree with Anton, this a debug feature and it should not > >> be added as an option, but simply should be removed from the codebase. > > > > It's not added as a regular option. It's an option that is only available > > when you compile with DEBUG defined. > > Ah, OK. > > > > > Isn't this an acceptable compromise? > > Well, I am not a fan of leaving DEBUG chunks in the codebase to be > honest. But if somebody applies it, then fine with me. The code exists in the code base for 6 years.. I was about to agree to the removal, but just few days ago I was glad that I could use it, and it might be useful in the near future in the context of subtitle filtering for troubleshooting. Also there's a bugfix patch where it could be useful: avcodec/dvdsubdec: fix incorrect yellow appearance of dvd subtitles https://github.com/ffstaging/FFmpeg/pull/16 https://master.gitmailbox.com/ffmpegdev/pull.16.ffstaging.ffmpeg.1641262759164.ffmpegag...@gmail.com Eventually, I'll submit a 'dumpgraphicsubs' filter for this purpose, and then that code can be removed anyway, but for the time being, I think this PR still improves the situation (no dump files written to disk unexpectedly), sufficiently enough to merge it even without a full removal. Thanks again, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
> -Original Message- > From: ffmpeg-devel On Behalf Of Marton > Balint > Sent: Friday, January 7, 2022 8:57 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't > dump images to disk based on DEBUG define > > > > On Fri, 7 Jan 2022, ffmpegagent wrote: > > > It's annoying and unexpected, but still useful at times (as I've realized > > just recently). > > > > This is a follow-up to the earlier submission here: > > https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html > > > > There has been a comment from Anton, questioning whether the dump-feature > is > > useful. Meanwhile I came to the conclusion that it can be useful in-fact. > It > > just shouldn't happen automatically when DEBUG is defined. That's what > these > > patches do. > > Well, I kind of agree with Anton, this a debug feature and it should not > be added as an option, but simply should be removed from the codebase. It's not added as a regular option. It's an option that is only available when you compile with DEBUG defined. Isn't this an acceptable compromise? softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't dump images to disk based on DEBUG define
> -Original Message- > From: ffmpeg-devel On Behalf Of Hendrik > Leppkes > Sent: Friday, January 7, 2022 11:32 AM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH 0/4] avcodec/dvbsubdec, dvdsubdec: don't > dump images to disk based on DEBUG define > > On Fri, Jan 7, 2022 at 11:20 AM Hendrik Leppkes wrote: > > > > On Fri, Jan 7, 2022 at 5:50 AM ffmpegagent wrote: > > > > > > It's annoying and unexpected, but still useful at times (as I've realized > > > just recently). > > > > > > This is a follow-up to the earlier submission here: > > > https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg128080.html > > > > > > There has been a comment from Anton, questioning whether the dump-feature > is > > > useful. Meanwhile I came to the conclusion that it can be useful in-fact. > It > > > just shouldn't happen automatically when DEBUG is defined. That's what > these > > > patches do. > > > > > > I also added fixes for the fopen() call. > > > > > > softworkz (4): > > > avcodec/dvbsubdec: don't dump images to disk based on DEBUG define > > > avcodec/dvbsubdec: fix writing ppm > > > avcodec/dvdsubdec: don't dump images to disk based on DEBUG define > > > avcodec/dvdsubdec: fix writing ppm > > > > > > libavcodec/dvbsubdec.c | 20 +--- > > > libavcodec/dvdsubdec.c | 11 --- > > > 2 files changed, 21 insertions(+), 10 deletions(-) > > > > > > > > > > These patches need some squasing. It should've been obvious from them > > being duplicated. > > > > Actually I fell into the dvb dvd trap, but regardless the patches are > basically identical, so I would still squash them, personally. No problem, but in which way? 1. All 4 commits into a single one 2. 4 >> 2 horizontally (a single one for the debug commits and a single one for the fopen commits) 3. 4 >> 2 vertically (a single commit for dvb and a single one for dvd) Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> -Original Message-
> From: Xiang, Haihao
> Sent: Friday, January 7, 2022 7:48 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: haihao.xiang-at-intel@ffmpeg.org; softwo...@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v3] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
>
> On Fri, 2022-01-07 at 03:58 +, ffmpegagent wrote:
> > From: softworkz
> >
> > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way that scaling via vpp_qsv doesn't work any longer
> > for devices with an MSDK runtime version lower than 1.19. This is true
> > for older CPUs which are stuck at 1.11.
> > The commit added checks for the compile-sdk version but it didn't test
> > for the runtime version.
> >
> > Signed-off-by: softworkz
> > ---
> > avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> >
> > Fix a recently introduced regression when using QSV VPP.
> >
> > v2: Fixed commit message wrapping v3: Use different way to acquire
> > runtime version
> >
> > Published-As:
> > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 15%2Fsoftworkz%2Fqsv_vpp_regression-v3
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > 15/softworkz/qsv_vpp_regression-v3
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> >
> > Range-diff vs v2:
> >
> > 1: ccede9d840 ! 1: cbf53d8ef4 avfilter/vpp_qsv: fix regression on older
> api
> > versions (e.g. 1.11)
> > @@ Commit message
> > Signed-off-by: softworkz
> >
> >## libavfilter/vf_vpp_qsv.c ##
> > +@@
> > + #include "libavutil/opt.h"
> > + #include "libavutil/eval.h"
> > + #include "libavutil/hwcontext.h"
> > ++#include "libavutil/hwcontext_qsv.h"
> > + #include "libavutil/pixdesc.h"
> > + #include "libavutil/mathematics.h"
> > +
> > @@ libavfilter/vf_vpp_qsv.c: static const AVOption options[] = {
> >{ "height", "Output video height", OFFSET(oh),
> AV_OPT_TYPE_STRING,
> > { .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> >{ "format", "Output pixel format", OFFSET(output_format_str),
> > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > @@ libavfilter/vf_vpp_qsv.c: static const AVOption options[] = {
> >{ NULL }
> >};
> >
> > +@@ libavfilter/vf_vpp_qsv.c: static int config_input(AVFilterLink
> > *inlink)
> > + return 0;
> > + }
> > +
> > ++static int get_mfx_version(const AVFilterContext *ctx, mfxVersion
> > *mfx_version)
> > ++{
> > ++const AVFilterLink *inlink = ctx->inputs[0];
> > ++AVBufferRef *device_ref;
> > ++AVHWDeviceContext *device_ctx;
> > ++AVQSVDeviceContext *device_hwctx;
> > ++
> > ++if (inlink->hw_frames_ctx) {
> > ++AVHWFramesContext *frames_ctx = (AVHWFramesContext *)inlink-
> > >hw_frames_ctx->data;
> > ++device_ref = frames_ctx->device_ref;
> > ++} else if (ctx->hw_device_ctx) {
> > ++device_ref = ctx->hw_device_ctx;
> > ++} else {
> > ++// Unavailable hw context doesn't matter in pass-through
> mode,
> > ++// so don't error here but let runtime version checks fail
> by
> > setting to 0.0
> > ++mfx_version->Major = 0;
> > ++mfx_version->Minor = 0;
> > ++return 0;
> > ++}
> > ++
> > ++device_ctx = (AVHWDeviceContext *)device_ref->data;
> > ++device_hwctx = device_ctx->hwctx;
> > ++
> > ++return MFXQueryVersion(device_hwctx->session, mfx_version);
>
> Thanks for the new patch version, it works well for me.
>
> It would be better to change the return type of get_mfx_version from int to
> mfxStatus if returning MFXQueryVersion(device_hwctx->session, mfx_version)
> here.
> Otherwise map the returned mfx status to ffmpeg status, then return ffmpeg
> status.
Yes, makes sense. Update on the way..
sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> -Original Message-
> From: Xiang, Haihao
> Sent: Tuesday, January 4, 2022 3:37 AM
> To: ffmpeg-devel@ffmpeg.org; softwo...@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
>
> On Tue, 2022-01-04 at 02:25 +, Soft Works wrote:
> > > -Original Message-
> > > From: Xiang, Haihao
> > > Sent: Tuesday, January 4, 2022 3:16 AM
> > > To: ffmpeg-devel@ffmpeg.org
> > > Cc: softwo...@hotmail.com
> > > Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression
> on
> > > older api versions (e.g. 1.11)
> > >
> > > On Sun, 2022-01-02 at 03:41 +, ffmpegagent wrote:
> > > > From: softworkz
> > > >
> > > > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > > regression in a way that scaling via vpp_qsv doesn't work any longer
> > > > for devices with an MSDK runtime version lower than 1.19. This is true
> > > > for older CPUs which are stuck at 1.11.
> > > > The commit added checks for the compile-sdk version but it didn't test
> > > > for the runtime version.
> > > >
> > > > Signed-off-by: softworkz
> > > > ---
> > > > avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> > > >
> > > > Fix a recently introduced regression when using QSV VPP.
> > > >
> > > > v2: Fixed commit message wrapping
> > > >
> > > > Published-As:
> > > > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> > >
> > > 15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> > > > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-
> > > > 15/softworkz/qsv_vpp_regression-v2
> > > > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> > > >
> > > > Range-diff vs v1:
> > > >
> > > > 1: eea966f702 ! 1: ccede9d840 avfilter/vpp_qsv: fix regression on
> older
> > >
> > > api
> > > > versions (e.g. 1.11)
> > > > @@ Metadata
> > > >## Commit message ##
> > > > avfilter/vpp_qsv: fix regression on older api versions (e.g.
> > >
> > > 1.11)
> > > >
> > > > -Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > > regression in a way
> > > > -that scaling via vpp_qsv didn't work any longer for devices
> with
> > >
> > > an
> > > > MSDK runtime
> > > > -version lower than 1.19. This is true for older CPUs which
> are
> > >
> > > stuck
> > > > at 1.11.
> > > > -The commit added checks for the compile-sdk version but it
> > > > didn't
> > > > test for the
> > > > -runtime version.
> > > > +Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > > > +regression in a way that scaling via vpp_qsv doesn't work any
> > >
> > > longer
> > > > +for devices with an MSDK runtime version lower than 1.19.
> This
> > > > is
> > > > true
> > > > +for older CPUs which are stuck at 1.11.
> > > > +The commit added checks for the compile-sdk version but it
> > > > didn't
> > > > test
> > > > +for the runtime version.
> > > >
> > > > Signed-off-by: softworkz
> > > >
> > > >
> > > >
> > > > libavfilter/vf_vpp_qsv.c | 32
> > > > 1 file changed, 20 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > > > index ab58a5777e..09590157e3 100644
> > > > --- a/libavfilter/vf_vpp_qsv.c
> > > > +++ b/libavfilter/vf_vpp_qsv.c
> > > > @@ -139,8 +139,9 @@ static const AVOption options[] = {
> > > > { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING,
> {
> > > > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> > > > { "format", "Output pixel format", OFFSET(output_format_str),
> > > > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > > > { "async_depth", "Internal parallelization depth, the higher the
>
Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> -Original Message-
> From: Xiang, Haihao
> Sent: Tuesday, January 4, 2022 3:16 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: softwo...@hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH v2] avfilter/vpp_qsv: fix regression on
> older api versions (e.g. 1.11)
>
> On Sun, 2022-01-02 at 03:41 +, ffmpegagent wrote:
> > From: softworkz
> >
> > Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way that scaling via vpp_qsv doesn't work any longer
> > for devices with an MSDK runtime version lower than 1.19. This is true
> > for older CPUs which are stuck at 1.11.
> > The commit added checks for the compile-sdk version but it didn't test
> > for the runtime version.
> >
> > Signed-off-by: softworkz
> > ---
> > avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)
> >
> > Fix a recently introduced regression when using QSV VPP.
> >
> > v2: Fixed commit message wrapping
> >
> > Published-As:
> > https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-
> 15%2Fsoftworkz%2Fqsv_vpp_regression-v2
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-
> > 15/softworkz/qsv_vpp_regression-v2
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/15
> >
> > Range-diff vs v1:
> >
> > 1: eea966f702 ! 1: ccede9d840 avfilter/vpp_qsv: fix regression on older
> api
> > versions (e.g. 1.11)
> > @@ Metadata
> >## Commit message ##
> > avfilter/vpp_qsv: fix regression on older api versions (e.g.
> 1.11)
> >
> > -Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > regression in a way
> > -that scaling via vpp_qsv didn't work any longer for devices with
> an
> > MSDK runtime
> > -version lower than 1.19. This is true for older CPUs which are
> stuck
> > at 1.11.
> > -The commit added checks for the compile-sdk version but it didn't
> > test for the
> > -runtime version.
> > +Commit 8b83dad82512a6948b63408f964463b063ad24c9 introduced a
> > +regression in a way that scaling via vpp_qsv doesn't work any
> longer
> > +for devices with an MSDK runtime version lower than 1.19. This is
> > true
> > +for older CPUs which are stuck at 1.11.
> > +The commit added checks for the compile-sdk version but it didn't
> > test
> > +for the runtime version.
> >
> > Signed-off-by: softworkz
> >
> >
> >
> > libavfilter/vf_vpp_qsv.c | 32
> > 1 file changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/libavfilter/vf_vpp_qsv.c b/libavfilter/vf_vpp_qsv.c
> > index ab58a5777e..09590157e3 100644
> > --- a/libavfilter/vf_vpp_qsv.c
> > +++ b/libavfilter/vf_vpp_qsv.c
> > @@ -139,8 +139,9 @@ static const AVOption options[] = {
> > { "height", "Output video height", OFFSET(oh), AV_OPT_TYPE_STRING, {
> > .str="w*ch/cw" }, 0, 255, .flags = FLAGS },
> > { "format", "Output pixel format", OFFSET(output_format_str),
> > AV_OPT_TYPE_STRING, { .str = "same" }, .flags = FLAGS },
> > { "async_depth", "Internal parallelization depth, the higher the value
> > the higher the latency.", OFFSET(async_depth), AV_OPT_TYPE_INT, { .i64 = 0
> },
> > 0, INT_MAX, .flags = FLAGS },
> > +#ifdef QSV_HAVE_SCALING_CONFIG
> > { "scale_mode", "scale mode: 0=auto, 1=low power, 2=high quality",
> > OFFSET(scale_mode), AV_OPT_TYPE_INT, { .i64 = MFX_SCALING_MODE_DEFAULT },
> > MFX_SCALING_MODE_DEFAULT, MFX_SCALING_MODE_QUALITY, .flags = FLAGS, "scale
> > mode" },
> > -
> > +#endif
> > { NULL }
> > };
> >
> > @@ -304,6 +305,7 @@ static int config_output(AVFilterLink *outlink)
> > QSVVPPParam param = { NULL };
> > QSVVPPCrop crop = { 0 };
> > mfxExtBuffer*ext_buf[ENH_FILTERS_COUNT];
> > +mfxVersion mfx_version;
> > AVFilterLink*inlink = ctx->inputs[0];
> > enum AVPixelFormat in_format;
> >
> > @@ -317,6 +319,11 @@ static int config_output(AVFilterLink *outlink)
> > param.ext_buf = ext_buf;
> > param.async_depth = vpp->async_depth;
> >
> > +if (MFXQueryVersion(vpp->qsv->session, _version) < 0) {
> > +av_log(ctx, AV_LOG_ERROR, "Failed to query mfx version.\n");
> > +return AVERROR(EINVAL);
> > +}
> > +
> > if (inlink->format == AV_PIX_FMT_QSV) {
> > if (!inlink->hw_frames_ctx || !inlink->hw_frames_ctx->data)
> > return AVERROR(EINVAL);
> > @@ -467,19 +474,20 @@ static int config_output(AVFilterLink *outlink)
> > #endif
> > }
> >
> > -if (inlink->w != outlink->w || inlink->h != outlink->h) {
> > #ifdef QSV_HAVE_SCALING_CONFIG
> > -memset(>scale_conf, 0, sizeof(mfxExtVPPScaling));
> > -vpp->scale_conf.Header.BufferId= MFX_EXTBUFF_VPP_SCALING;
> > -vpp->scale_conf.Header.BufferSz= sizeof(mfxExtVPPScaling);
> > -vpp->scale_conf.ScalingMode= vpp->scale_mode;
> > -
> > -param.ext_buf[param.num_ext_buf++]
Re: [FFmpeg-devel] 5.0 release
> -Original Message- > From: ffmpeg-devel On Behalf Of Jean- > Baptiste Kempf > Sent: Monday, January 3, 2022 6:32 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] 5.0 release > > On Fri, 31 Dec 2021, at 20:40, Michael Niedermayer wrote: > >> It would be nice to have a public date set a few days into the future. > > > > yes, i intended to do that, unless people wanted a ASAP/NOW branch > > So what are the open topics, besides the Audio Channel Layout API? If you don't want to ship a regression: [PATCH v2] avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11) Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [RFC] QSV: Introduce min Compile-SDK Version and check for Runtime-Versions instead
Hi, this is a follow-up to my recently submitted patch: “avfilter/vpp_qsv: fix regression on older api versions (e.g. 1.11)” That patch only fixes the one important regression from multiple issues which have been introduced by recent changes. Those changes have gone in a less than ideal direction, because they have introduced checks based on the version of the MSDK which ffmpeg is being compiled against, but they didn’t add checks for the runtime libmfx/MSDK versions – and this causes failures, sometimes even for functionality which has worked before (=>regressions). Background - the compile-sdk version determines which features can be used but only when - the runtime SDK version supports it and - the hardware (GPU gen) supports it - ALL compile-sdk versions can interface with ALL runtime MSDK versions (basically), no matter whether runtime version is newer or older - At least on Windows, some hw is stuck at a certain runtime version, e.g.: there are new drivers for Broadwell, but the MSDK runtime is always 1.11 Conclusion Adding checks for the runtime MSDK versions is required wherever a feature might not be supported by older MSDK runtimes - I think that's an obvious necessity. Question Having both - run-time and compile-time checks all over the code is adding a lot of complexity and makes it difficult to maintain and work with. Hence, I'm wondering whether we couldn't/shouldn't introduce a minimum MSDK compile-time version, for example 1.22, or even later? This would allow simplification of the QSV code in many places where run-time version checks are actually needed instead. Over time, there have been better and worse MSDK versions, and there should still be enough room for choosing, but I don't think there's any reason why somebody would still want to compile against some really old (e.g. < 1.22) MSDK version. Please share your thoughts on this subject.. softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Lynne > Sent: Sunday, January 2, 2022 3:04 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] GitHub Integration > > 2 Jan 2022, 04:28 by softwo...@hotmail.com: > > > > > > >> -Original Message----- > >> From: ffmpeg-devel On Behalf Of Soft > Works > >> Sent: Tuesday, December 28, 2021 10:58 PM > >> To: FFmpeg development discussions and patches > >> Subject: Re: [FFmpeg-devel] GitHub Integration > >> > >> One component of the GitHub "Bridge" is a mirror of the ffmpeg-devel > >> mailing list as a Git repository which provides a web interfaces, can be > >> cloned via Git and can also be accessed from Atom feed readers. > >> This allows to follow the mailing list without subscribing. The web UI > >> might not be everybody's taste, though. It's the same that is used for > >> Linux kernel mailing lists (https://lore.kernel.org/git/). > >> I had to set this up as it's a requirement for the GitHub "Bridge", but > >> maybe it's useful for someone in other ways: > >> > >> https://master.gitmailbox.com/ffmpegdev/ > >> > > > > > > I have just ACTIVATED patch submission via GitHub and submitted the first > > patch through this method. > > > > Pull requests can be created in this repository (for now): > > > > https://github.com/ffstaging/FFmpeg > > > > > > What's not nice is that the submitted e-mail shows "ffmpegagent" as sender > > and patchwork is showing it under that name as well. > > > > We'll need to see how this can be improved. > > > > I don't like it. Can't it use the author's name for emails? I don't like this either. I'll try to find a way. sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Soft Works > Sent: Tuesday, December 28, 2021 10:58 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] GitHub Integration > > One component of the GitHub "Bridge" is a mirror of the ffmpeg-devel > mailing list as a Git repository which provides a web interfaces, can be > cloned via Git and can also be accessed from Atom feed readers. > This allows to follow the mailing list without subscribing. The web UI > might not be everybody's taste, though. It's the same that is used for > Linux kernel mailing lists (https://lore.kernel.org/git/). > I had to set this up as it's a requirement for the GitHub "Bridge", but > maybe it's useful for someone in other ways: > > https://master.gitmailbox.com/ffmpegdev/ I have just ACTIVATED patch submission via GitHub and submitted the first patch through this method. Pull requests can be created in this repository (for now): https://github.com/ffstaging/FFmpeg What's not nice is that the submitted e-mail shows "ffmpegagent" as sender and patchwork is showing it under that name as well. We'll need to see how this can be improved. Happy new year! softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Thursday, December 30, 2021 12:22 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 30/12/2021 00:29, Soft Works wrote: > >> -Original Message- > >> From: ffmpeg-devel On Behalf Of Mark > >> Thompson > >> Sent: Thursday, December 30, 2021 12:04 AM > >> To: ffmpeg-devel@ffmpeg.org > >> Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When > deriving a > >> hwdevice, search for existing device in both directions > >> > >> On 25/11/2021 02:41, Soft Works wrote: > >>> The test /libavutil/tests/hwdevice checks that when deriving a device > >>> from a source device and then deriving back to the type of the source > >>> device, the result is matching the original source device, i.e. the > >>> derivation mechanism doesn't create a new device in this case. > >>> > >>> Previously, this test was usually passed, but only due to two different > >>> kind of flaws: > >>> > >>> 1. The test covers only a single level of derivation (and back) > >>> > >>> It derives device Y from device X and then Y back to the type of X and > >>> checks whether the result matches X. > >>> > >>> What it doesn't check for, are longer chains of derivation like: > >>> > >>> CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4 > >>> > >>> In that case, the second derivation returns the first device (CUDA3 == > >>> CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new > >>> OpenCL4 context instead of returning OpenCL2, because there was no link > >>> from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1) > >> > >> Yes, this is exactly what I expect. > >> > >> Because of how these APIs work, device derivation is always one-way - you > can > >> make an OpenCL device from a D3D11 one, but not the other direction. I > don't > >> think there is any case which allows both directions > > > > hwmap=reverse=1 > > Indeed, hwmap reverse exists because mapping is one-way and sometimes a > filter graph wants to use it in the other direction. => mapping is not only one-way > >> Saying that derivation from A should always return the same B is not > >> intended, nor do I think it should be. > > > > Why not? > > > > Looking at the reality of API users: > > > > - I'm covering a wide range of different processing pipelines and > >found that this behavior is crucial to make important and relevant > >processing pipelines work > > > > - Intel have three different workaround-patches in their backlog/queue > >of ffmpeg patches to get certain processing setups working > > > > - The developers working on Vulkan have confirmed that this change > >is necessary and crucial for certain setups to work > > > > - Nobody has named any case or scenario that would be negatively > >affected by this change > > > > Given that situation, I don't think it's useful to talk about > > theoretical implications. > > You are not talking about API users at all. When does an API user ever want > this patch? From their point of view it is surprising and unwanted - if they > want the same device again, they just use the same device again. No. Not when using libavfilter with a filter graph. => It affects API users in the same way > > You are talking about users of the ffmpeg utility. No. See above. > The change is a library > hack to work around the inability to select devices per-filter in the ffmpeg > utility. No, it's not a hack, it's about making libavfilter hw context deriving and reverse mapping working as expected and needed. > Please, just implement device selection for filters in ffmpeg rather than > adding unexpected behaviour elsewhere. libavfilter has supported it for API > users for a long time, no library changes should be needed. I'm afraid but I disagree. I stick to the proposed solution; this change is welcomed and needed by many which are having problems with the current implementation. I still haven't seen any (real-world and not theoretical constructed) scenario where this would have a negative effect or break something, neither did I see anybody else objecting. Anyway, I have no intentions to propose a different solution. This is what I'm using and what others want and need. Thanks, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions
> -Original Message- > From: ffmpeg-devel On Behalf Of Mark > Thompson > Sent: Thursday, December 30, 2021 12:04 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [PATCH v4 1/1] avutils/hwcontext: When deriving a > hwdevice, search for existing device in both directions > > On 25/11/2021 02:41, Soft Works wrote: > > The test /libavutil/tests/hwdevice checks that when deriving a device > > from a source device and then deriving back to the type of the source > > device, the result is matching the original source device, i.e. the > > derivation mechanism doesn't create a new device in this case. > > > > Previously, this test was usually passed, but only due to two different > > kind of flaws: > > > > 1. The test covers only a single level of derivation (and back) > > > > It derives device Y from device X and then Y back to the type of X and > > checks whether the result matches X. > > > > What it doesn't check for, are longer chains of derivation like: > > > > CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4 > > > > In that case, the second derivation returns the first device (CUDA3 == > > CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new > > OpenCL4 context instead of returning OpenCL2, because there was no link > > from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1) > > Yes, this is exactly what I expect. > > Because of how these APIs work, device derivation is always one-way - you can > make an OpenCL device from a D3D11 one, but not the other direction. I don't > think there is any case which allows both directions hwmap=reverse=1 > Saying that derivation from A should always return the same B is not > intended, nor do I think it should be. Why not? Looking at the reality of API users: - I'm covering a wide range of different processing pipelines and found that this behavior is crucial to make important and relevant processing pipelines work - Intel have three different workaround-patches in their backlog/queue of ffmpeg patches to get certain processing setups working - The developers working on Vulkan have confirmed that this change is necessary and crucial for certain setups to work - Nobody has named any case or scenario that would be negatively affected by this change Given that situation, I don't think it's useful to talk about theoretical implications. Kind regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug for mapping qsv frame to opencl
> -Original Message-
> From: ffmpeg-devel On Behalf Of Mark
> Thompson
> Sent: Thursday, December 30, 2021 12:23 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug
> for mapping qsv frame to opencl
>
> On 28/12/2021 19:04, Soft Works wrote:>> -Original Message-
> >> From: ffmpeg-devel On Behalf Of Mark
> >> Thompson
> >> Sent: Tuesday, December 28, 2021 1:54 PM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a
> bug
> >> for mapping qsv frame to opencl
> >>
> >> On 28/12/2021 01:17, Soft Works wrote:
> >>>> -Original Message-
> >>>> From: ffmpeg-devel On Behalf Of Mark
> >>>> Thompson
> >>>> Sent: Tuesday, December 28, 2021 12:46 AM
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix
> a
> >> bug
> >>>> for mapping qsv frame to opencl
> >>>>
> >>>> On 27/12/2021 20:31, Soft Works wrote:>> -Original Message-
> >>>>>> From: ffmpeg-devel On Behalf Of Mark
> >>>>>> Thompson
> >>>>>> Sent: Monday, December 27, 2021 7:51 PM
> >>>>>> To: ffmpeg-devel@ffmpeg.org
> >>>>>> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl:
> fix
> >> a
> >>>> bug
> >>>>>> for mapping qsv frame to opencl
> >>>>>>
> >>>>>> On 16/11/2021 08:16, Wenbin Chen wrote:
> >>>>>>> From: nyanmisaka
> >>>>>>>
> >>>>>>> mfxHDLPair was added to qsv, so modify qsv->opencl map function as
> >> well.
> >>>>>>> Now the following commandline works:
> >>>>>>>
> >>>>>>> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128 \
> >>>>>>> -init_hw_device qsv=qs@va -init_hw_device opencl=ocl@va -
> >> filter_hw_device
> >>>>>> ocl \
> >>>>>>> -hwaccel qsv -hwaccel_output_format qsv -hwaccel_device qs -c:v
> >> h264_qsv
> >>>> \
> >>>>>>> -i input.264 -vf
> >>>> "hwmap=derive_device=opencl,format=opencl,avgblur_opencl,
> >>>>>> \
> >>>>>>> hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32,format=qsv" \
> >>>>>>> -c:v h264_qsv output.264
> >>>>>>>
> >>>>>>> Signed-off-by: nyanmisaka
> >>>>>>> Signed-off-by: Wenbin Chen
> >>>>>>> ---
> >>>>>>> libavutil/hwcontext_opencl.c | 3 ++-
> >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/libavutil/hwcontext_opencl.c
> >> b/libavutil/hwcontext_opencl.c
> >>>>>>> index 26a3a24593..4b6e74ff6f 100644
> >>>>>>> --- a/libavutil/hwcontext_opencl.c
> >>>>>>> +++ b/libavutil/hwcontext_opencl.c
> >>>>>>> @@ -2249,7 +2249,8 @@ static int
> opencl_map_from_qsv(AVHWFramesContext
> >>>>>> *dst_fc, AVFrame *dst,
> >>>>>>> #if CONFIG_LIBMFX
> >>>>>>> if (src->format == AV_PIX_FMT_QSV) {
> >>>>>>> mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src-
> >>>>> data[3];
> >>>>>>> -va_surface = *(VASurfaceID*)mfx_surface->Data.MemId;
> >>>>>>> +mfxHDLPair *pair = (mfxHDLPair*)mfx_surface->Data.MemId;
> >>>>>>> +va_surface = *(VASurfaceID*)pair->first;
> >>>>>>> } else
> >>>>>>> #endif
> >>>>>>> if (src->format == AV_PIX_FMT_VAAPI) {
> >>>>>>
> >>>>>> Since these frames can be user-supplied, this implies that the user-
> >> facing
> >>>>>> API/ABI for AV_PIX_FMT_QSV has changed.
> >>>>>>
> >>>>>> It looks like this was broken by using HDLPairs when D3D11 was
> >> introduced,
> >>>>>> which silently changed the existing API for DXVA2 and VAAPI as
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Niklas Haas > Sent: Tuesday, December 28, 2021 10:22 PM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] GitHub Integration > > On Tue, 28 Dec 2021 00:59:58 +1000 Zane van Iperen > wrote: > > > > > > On 27/12/21 11:41, lance.lmw...@gmail.com wrote: > > > On Sun, Dec 26, 2021 at 04:37:54PM -0500, Ronald S. Bultje wrote: > > >> Hi, > > >> > > >> On Sun, Dec 26, 2021 at 3:21 PM Soft Works > wrote: > > >> > > >>> I'm not sure. My interpretation of Lance' and Steven's comments would > > >>> be that they'd prefer to stick to the ML. > > >>> > > >> > > >> No, it's not strictly related to that - they want something that is CLI > > >> accessible. Gitlab has this here: https://glab.readthedocs.io/en/latest/ > > >> and github has this here: https://github.com/cli/cli - the next question > is > > >> whether the gitlab/hub hosts are blocked by a firewall (no idea) and/or > > >> whether the instances are self-hosted (github: no, gitlab-videolan: > yes). > > > > > > Yes, self-hosted is more preferable, I recall github has blocked > devleopers > > > in some country by US trade controls. Who knows what's the rules will be > > > changed someday as it's controlled by company. > > > > > > > Something that doesn't require another account would be nice, which is why > > I like mailing lists. > > I don't understand, isn't this an argument in favor of GitHub? Most > mailing lists (including, notably, FFmpeg's) require registration in > order to submit messages, which is one of the reasons I hate them. It's > not just that registration is annoying in general, it's also that > registration to a mailing list is *more* annoying and cumbersome than > creating an account on any post-90s website. > > Conversely on GitHub, everybody already has an account, so the overhead > for first-time contribution is *actually* zero. And on every other > self-hosted GitLab instance I've come across, I could cross-authenticate > with some other account I already have. One component of the GitHub "Bridge" is a mirror of the ffmpeg-devel mailing list as a Git repository which provides a web interfaces, can be cloned via Git and can also be accessed from Atom feed readers. This allows to follow the mailing list without subscribing. The web UI might not be everybody's taste, though. It's the same that is used for Linux kernel mailing lists (https://lore.kernel.org/git/). I had to set this up as it's a requirement for the GitHub "Bridge", but maybe it's useful for someone in other ways: https://master.gitmailbox.com/ffmpegdev/ Regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug for mapping qsv frame to opencl
> -Original Message-
> From: ffmpeg-devel On Behalf Of Mark
> Thompson
> Sent: Tuesday, December 28, 2021 1:54 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug
> for mapping qsv frame to opencl
>
> On 28/12/2021 01:17, Soft Works wrote:
> >> -Original Message-
> >> From: ffmpeg-devel On Behalf Of Mark
> >> Thompson
> >> Sent: Tuesday, December 28, 2021 12:46 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a
> bug
> >> for mapping qsv frame to opencl
> >>
> >> On 27/12/2021 20:31, Soft Works wrote:>> -Original Message-
> >>>> From: ffmpeg-devel On Behalf Of Mark
> >>>> Thompson
> >>>> Sent: Monday, December 27, 2021 7:51 PM
> >>>> To: ffmpeg-devel@ffmpeg.org
> >>>> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix
> a
> >> bug
> >>>> for mapping qsv frame to opencl
> >>>>
> >>>> On 16/11/2021 08:16, Wenbin Chen wrote:
> >>>>> From: nyanmisaka
> >>>>>
> >>>>> mfxHDLPair was added to qsv, so modify qsv->opencl map function as
> well.
> >>>>> Now the following commandline works:
> >>>>>
> >>>>> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128 \
> >>>>> -init_hw_device qsv=qs@va -init_hw_device opencl=ocl@va -
> filter_hw_device
> >>>> ocl \
> >>>>> -hwaccel qsv -hwaccel_output_format qsv -hwaccel_device qs -c:v
> h264_qsv
> >> \
> >>>>> -i input.264 -vf
> >> "hwmap=derive_device=opencl,format=opencl,avgblur_opencl,
> >>>> \
> >>>>> hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32,format=qsv" \
> >>>>> -c:v h264_qsv output.264
> >>>>>
> >>>>> Signed-off-by: nyanmisaka
> >>>>> Signed-off-by: Wenbin Chen
> >>>>> ---
> >>>>> libavutil/hwcontext_opencl.c | 3 ++-
> >>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/libavutil/hwcontext_opencl.c
> b/libavutil/hwcontext_opencl.c
> >>>>> index 26a3a24593..4b6e74ff6f 100644
> >>>>> --- a/libavutil/hwcontext_opencl.c
> >>>>> +++ b/libavutil/hwcontext_opencl.c
> >>>>> @@ -2249,7 +2249,8 @@ static int opencl_map_from_qsv(AVHWFramesContext
> >>>> *dst_fc, AVFrame *dst,
> >>>>> #if CONFIG_LIBMFX
> >>>>> if (src->format == AV_PIX_FMT_QSV) {
> >>>>> mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src-
> >>> data[3];
> >>>>> -va_surface = *(VASurfaceID*)mfx_surface->Data.MemId;
> >>>>> +mfxHDLPair *pair = (mfxHDLPair*)mfx_surface->Data.MemId;
> >>>>> +va_surface = *(VASurfaceID*)pair->first;
> >>>>> } else
> >>>>> #endif
> >>>>> if (src->format == AV_PIX_FMT_VAAPI) {
> >>>>
> >>>> Since these frames can be user-supplied, this implies that the user-
> facing
> >>>> API/ABI for AV_PIX_FMT_QSV has changed.
> >>>>
> >>>> It looks like this was broken by using HDLPairs when D3D11 was
> introduced,
> >>>> which silently changed the existing API for DXVA2 and VAAPI as well.
> >>>>
> >>>> Could someone related to that please document it properly (clearly not
> all
> >>>> possible valid mfxFrameSurface1s are allowed), and note in APIchanges
> when
> >>>> the API change happened?
> >>>
> >>> Hi Mark,
> >>>
> >>> QSV contexts always need to be backed by a child context, which can be
> >> DXVA2,
> >>> D3D11VA or VAAPI. You can create a QSV context either by deriving from
> one
> >> of
> >>> those contexts or when create a new QSV context, it automatically creates
> >> an
> >>> appropriate child context - either implicitly (auto mode) or explicitly,
> >> like
> >>> the ffmpeg implementation does in most cases.
> >>
> >> ... or by using the one the user supplies when they create it.
> >>
> >>> When working with "
Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug for mapping qsv frame to opencl
> -Original Message-
> From: ffmpeg-devel On Behalf Of Mark
> Thompson
> Sent: Tuesday, December 28, 2021 12:46 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug
> for mapping qsv frame to opencl
>
> On 27/12/2021 20:31, Soft Works wrote:>> -Original Message-
> >> From: ffmpeg-devel On Behalf Of Mark
> >> Thompson
> >> Sent: Monday, December 27, 2021 7:51 PM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a
> bug
> >> for mapping qsv frame to opencl
> >>
> >> On 16/11/2021 08:16, Wenbin Chen wrote:
> >>> From: nyanmisaka
> >>>
> >>> mfxHDLPair was added to qsv, so modify qsv->opencl map function as well.
> >>> Now the following commandline works:
> >>>
> >>> ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128 \
> >>> -init_hw_device qsv=qs@va -init_hw_device opencl=ocl@va -filter_hw_device
> >> ocl \
> >>> -hwaccel qsv -hwaccel_output_format qsv -hwaccel_device qs -c:v h264_qsv
> \
> >>> -i input.264 -vf
> "hwmap=derive_device=opencl,format=opencl,avgblur_opencl,
> >> \
> >>> hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32,format=qsv" \
> >>> -c:v h264_qsv output.264
> >>>
> >>> Signed-off-by: nyanmisaka
> >>> Signed-off-by: Wenbin Chen
> >>> ---
> >>>libavutil/hwcontext_opencl.c | 3 ++-
> >>>1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> >>> index 26a3a24593..4b6e74ff6f 100644
> >>> --- a/libavutil/hwcontext_opencl.c
> >>> +++ b/libavutil/hwcontext_opencl.c
> >>> @@ -2249,7 +2249,8 @@ static int opencl_map_from_qsv(AVHWFramesContext
> >> *dst_fc, AVFrame *dst,
> >>>#if CONFIG_LIBMFX
> >>>if (src->format == AV_PIX_FMT_QSV) {
> >>>mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src-
> >data[3];
> >>> -va_surface = *(VASurfaceID*)mfx_surface->Data.MemId;
> >>> +mfxHDLPair *pair = (mfxHDLPair*)mfx_surface->Data.MemId;
> >>> +va_surface = *(VASurfaceID*)pair->first;
> >>>} else
> >>>#endif
> >>>if (src->format == AV_PIX_FMT_VAAPI) {
> >>
> >> Since these frames can be user-supplied, this implies that the user-facing
> >> API/ABI for AV_PIX_FMT_QSV has changed.
> >>
> >> It looks like this was broken by using HDLPairs when D3D11 was introduced,
> >> which silently changed the existing API for DXVA2 and VAAPI as well.
> >>
> >> Could someone related to that please document it properly (clearly not all
> >> possible valid mfxFrameSurface1s are allowed), and note in APIchanges when
> >> the API change happened?
> >
> > Hi Mark,
> >
> > QSV contexts always need to be backed by a child context, which can be
> DXVA2,
> > D3D11VA or VAAPI. You can create a QSV context either by deriving from one
> of
> > those contexts or when create a new QSV context, it automatically creates
> an
> > appropriate child context - either implicitly (auto mode) or explicitly,
> like
> > the ffmpeg implementation does in most cases.
>
> ... or by using the one the user supplies when they create it.
>
> > When working with "user-supplied" frames on Linux, you need to create a
> VAAPI
> > context with those frames and derive a QSV context from that context.
> >
> > There is no way to create or supply QSV frames directly.
>
> ??? The ability for the user to set up their own version of these things is
> literally the whole point of the split alloc/init API.
>
>
> // Some user stuff involving libmfx - has a D3D or VAAPI backing, but this
> code doesn't need to care about it.
>
> // It has a session and creates some surfaces to use with MemId filled
> compatible with ffmpeg.
> user_session = ...;
> user_surfaces = ...;
>
> // No ffmpeg involved before this, now we want to pass these surfaces we've
> got into ffmpeg.
>
> // Create a device context using the existing session.
>
> mfx_ctx = av_hwdevice_ctx_alloc(MFX);
>
> dc = mfx_ctx->data;
> mfx_dc = dc->hwctx;
> mfx_dc->session = user_session;
>
> av_hwdevice_ctx_init(mfx_ctx);
>
> // Create a frames context out of t
Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug for mapping qsv frame to opencl
> -Original Message-
> From: ffmpeg-devel On Behalf Of Mark
> Thompson
> Sent: Monday, December 27, 2021 7:51 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 3/3] libavutil/hwcontext_opencl: fix a bug
> for mapping qsv frame to opencl
>
> On 16/11/2021 08:16, Wenbin Chen wrote:
> > From: nyanmisaka
> >
> > mfxHDLPair was added to qsv, so modify qsv->opencl map function as well.
> > Now the following commandline works:
> >
> > ffmpeg -v verbose -init_hw_device vaapi=va:/dev/dri/renderD128 \
> > -init_hw_device qsv=qs@va -init_hw_device opencl=ocl@va -filter_hw_device
> ocl \
> > -hwaccel qsv -hwaccel_output_format qsv -hwaccel_device qs -c:v h264_qsv \
> > -i input.264 -vf "hwmap=derive_device=opencl,format=opencl,avgblur_opencl,
> \
> > hwmap=derive_device=qsv:reverse=1:extra_hw_frames=32,format=qsv" \
> > -c:v h264_qsv output.264
> >
> > Signed-off-by: nyanmisaka
> > Signed-off-by: Wenbin Chen
> > ---
> > libavutil/hwcontext_opencl.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> > index 26a3a24593..4b6e74ff6f 100644
> > --- a/libavutil/hwcontext_opencl.c
> > +++ b/libavutil/hwcontext_opencl.c
> > @@ -2249,7 +2249,8 @@ static int opencl_map_from_qsv(AVHWFramesContext
> *dst_fc, AVFrame *dst,
> > #if CONFIG_LIBMFX
> > if (src->format == AV_PIX_FMT_QSV) {
> > mfxFrameSurface1 *mfx_surface = (mfxFrameSurface1*)src->data[3];
> > -va_surface = *(VASurfaceID*)mfx_surface->Data.MemId;
> > +mfxHDLPair *pair = (mfxHDLPair*)mfx_surface->Data.MemId;
> > +va_surface = *(VASurfaceID*)pair->first;
> > } else
> > #endif
> > if (src->format == AV_PIX_FMT_VAAPI) {
>
> Since these frames can be user-supplied, this implies that the user-facing
> API/ABI for AV_PIX_FMT_QSV has changed.
>
> It looks like this was broken by using HDLPairs when D3D11 was introduced,
> which silently changed the existing API for DXVA2 and VAAPI as well.
>
> Could someone related to that please document it properly (clearly not all
> possible valid mfxFrameSurface1s are allowed), and note in APIchanges when
> the API change happened?
Hi Mark,
QSV contexts always need to be backed by a child context, which can be DXVA2,
D3D11VA or VAAPI. You can create a QSV context either by deriving from one of
those contexts or when create a new QSV context, it automatically creates an
appropriate child context - either implicitly (auto mode) or explicitly, like
the ffmpeg implementation does in most cases.
When working with "user-supplied" frames on Linux, you need to create a VAAPI
context with those frames and derive a QSV context from that context.
There is no way to create or supply QSV frames directly. Looking at the code:
> *mfx_surface = (mfxFrameSurface1*)src->data[3];
A QSV frames context is using the mfxFrameSurface1 structure for describing
the individual frames and mfxFrameSurface1 can only come from the MSDK runtime,
it cannot be user-supplied.
I don't think that there's something that needs to be documented because
whatever user-side manipulation an API consumer would want to perform, it would
always need to derive the context either from QSV to D3D/VAAPI or from D3D to
VAAPI in order to access and manipulate individual frames.
Kind regards,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Ronald S. > Bultje > Sent: Sunday, December 26, 2021 10:38 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] GitHub Integration > > Hi, > > On Sun, Dec 26, 2021 at 3:21 PM Soft Works wrote: > > > I'm not sure. My interpretation of Lance' and Steven's comments would > > be that they'd prefer to stick to the ML. > > > > No, it's not strictly related to that - they want something that is CLI > accessible. Gitlab has this here: https://glab.readthedocs.io/en/latest/ > and github has this here: https://github.com/cli/cli - the next question is > whether the gitlab/hub hosts are blocked by a firewall (no idea) and/or > whether the instances are self-hosted (github: no, gitlab-videolan: yes). Agreed. For the submission part, there are CLI options in both cases. Also true for both cases: the discussion part would change to WWW. But I'm probably the very last one here to advocate staying with the ML. Even though I'd prefer GitHub, I would welcome a move to GitLab as well, and when something like GitGitGadget would have existed for GitLab instead of GitHub, then I would have adopted that instead. Looking back at the discussion in August, where I made these suggestions, a lot of developers had responded, indicating their satisfaction with the ML approach (some even defending it strongly). Just very few indicated support for a move to either GitHub or GitLab. Off-list, I've seen some who were claiming that a majority would exist, favoring a move to GitLab. That would be great, but I'm not seeing it. Where is that majority and why do so few of them speak up, then? After reading the ML for several years, and from what I've learned about the group interaction dynamics, my assessment is, that a migration away from the ML is unlikely to happen in a short-term future. Those who don't want to move are just on standby, waiting for the next attempt to be made in order to turn it down once again, and those who would like to move, are sufficiently satisfied by the thought alone that a move to GitLab would happen "soon". Most likely, by the end of 2022, the situation will just be the same. On the other side, the "GitHub Gateway/Integration" with the ffmpeg-codebot is available right now and able to bridge the gap between ML and more modern and integrated development workflows. It's not meant to replace, avoid or compete with an actual migration, and when a migration would happen e.g. to GitLab, it will be still useful to have a bot on GitHub that either mirrors PRs from GitHub or just auto-closes them with an appropriate message. Somebody said on IRC that the GH integration might only delay a real migration away from the ML. But I'm seeing it somewhat differently: this might rather serve as an indication for which approach will be preferred by the majority, maybe even accelerating a final migration. Kind regards, softworkz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
> -Original Message- > From: ffmpeg-devel On Behalf Of Lynne > Sent: Sunday, December 26, 2021 5:49 PM > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] GitHub Integration > > 26 Dec 2021, 02:03 by l...@chinaffmpeg.org: > > > > > > >> 在 2021年12月26日，上午8:55，lance.lmw...@gmail.com 写道： > >> > >> On Sat, Dec 25, 2021 at 05:15:19PM +, Soft Works wrote: > >> > >>> > >>> > >>>> -Original Message- > >>>> From: ffmpeg-devel On Behalf Of Lynne > >>>> Sent: Saturday, December 25, 2021 5:50 PM > >>>> To: FFmpeg development discussions and patches > >>>> Subject: Re: [FFmpeg-devel] GitHub Integration > >>>> > >>>> 23 Dec 2021, 00:24 by softwo...@hotmail.com: > >>>> > >>>>> Hi, > >>>>> > >>>>> holidays are approaching and I got a little present for all of you > >>>>> even though it won’t be something for everybody. > >>>>> > >>>>> A while ago I had committed to prepare a test setup for integrating > >>>>> GitHub in a similar way as the Git developers are doing. > >>>>> > >>>>> For those who missed it or don’t remember the outcome: > >>>>> https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg123329.html > >>>>> https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg123288.html > >>>>> > >>>>> Before someone’s temperature might start rising, I want to repeat > >>>>> some important corner points: > >>>>> > >>>>> - it's not a migration nor the start of a migration > >>>>> - nothing changes, it doesn't affect anybody's way of working > >>>>> it doesn't replace anything > >>>>> - it's an add-on that one could use or not > >>>>> > >>>>> Basic functionality is that it allows to submit patches to the ML > >>>>> through GitHub pull requests. > >>>>> > >>>>> Right now must features are working, but e-mails are not sent to > >>>>> the ML yet, just to one's own e-mail address. > >>>>> > >>>>> > >>>>> I don't want to post the repository location publicly at this time, > >>>>> but for anybody who would be interested in taking a look at this, > >>>>> please send me an e-mail or PM me on IRC. > >>>>> > >>>> > >>>> I'd rather we move away from ML development altogether. > >>>> There's general consensus already, and of course one or > >>>> two who'd rather not. > >>>> > >>> > >>> I'd prefer that as well, but in back in summer there were quite a number > >>> of developers (> 2) indicating that they are fine with the > >>> ML approach, so I'm not really sure about whether and when this > >>> will happen eventually. > >>> > >>> This GitHub-PR-Bridge is not meant to be an alternative or replacement > >>> for an eventual full migration to another platform. > >>> > >>> It's just an interim solution until a migration will eventually happen > >>> (probably like in the 2030ies.. ;-) > >>> > >> > >> My point: > >> 1, In China network environment, when accessing Github, the network will > be > >> blocked or very slow. > >> 2, For gmail is blocked, so I prefer to use cli, remote ssh to a cloud > host, > >> use mutt/vim for ML email, and scp, and git cli tools are professional to > do > >> everything for the patchset send, push etc. Yes, I can use socket5 to > forward, > >> but I like cli instead of web for I prefer to use keyboard instead of > mouse. > >> > > > > +1 > > > > We'd never move to Github, that much's for sure. Videolan already > hosts a Gitlab instance, and I doubt the great firewall would have an > issue with that. I'm not sure. My interpretation of Lance' and Steven's comments would be that they'd prefer to stick to the ML. sw ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
