Re: [FFmpeg-devel] [PATCH V2 12/12] dnn-layer-math-unary-test: add unit test for atanh
> -Original Message- > From: ffmpeg-devel On Behalf Of Ting Fu > Sent: 2020年6月29日 22:54 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH V2 12/12] dnn-layer-math-unary-test: add unit > test for atanh > > Signed-off-by: Ting Fu > --- > tests/dnn/dnn-layer-mathunary-test.c | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) this patch set looks good to me, will push soon, 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] Project orientation
> -Original Message- > From: ffmpeg-devel On Behalf Of Jim > DeLaHunt > Sent: Monday, July 6, 2020 6:54 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] Project orientation > > On 2020-07-04 07:43, Nicolas George wrote: > > Hi. > > > > When I first started progressively to contribute do FFmpeg, the project was > > far from mature, but it was very much alive. > > > > There was attempts at implementing new and better codecs, directly in lavc's > > code base. There were attempts at designing new formats with useful > > features. > > > > Even outside the specialized work on specific codecs and formats, there was > > creativity. At the API and framework level, there was work being done to do > > things in a smarter way, either more convenient or more efficient or both. > > > > For example, the format negotiation in liabvfilter (which was mostly > > designed > > before I got involved: I am not singing my own praise here), with its > > pointers > > to pointers to pointers, is not run-of-the-mill code. > > Even if parts of it are made of common algorithms, the whole is very > > specific > > to FFmpeg, and its design was creative. Creativity was necessary later to > > extend it to support audio. > > > > Nowadays, not so much. There is work in making highly-optimized decoders, > > this work is impressive and creative, there is no doubt about it. But as > > far as I > > can see, that is mostly all there is going on. The rest seems to be rather > > basic: > > fixing bugs (and things that are not bugs, like harmless integer overflows), > > implementing support for features that were decided and designed > > elsewhere. > > > > And it is not just that it is not happening: there is genuine opposition to > > creativity: things that are not already justified externally, things that > > do not > > look like well-known patterns, are met with scepticism and eventually > > rejected. > > > > At this point, we should ask ourselves: > > > > Is it what we want the project to be, or is it just what we have let it > > become? > > > > I do not think this evolution is the result of a deliberate choice. I think > > it is > > more the result of the stress of success and the stress of a fork. Success > > can > > stifle creativity, because creativity implies the risk of failure: the > > project has > > become advert to risk. > > > > It has evolved that way, but are we fine with it? > > > > I personally am not. > > > > I find the new ambiance boring. > > > > Creativity is the reason I practice development, and the reason I do not > > practice it professionally: my creativity cannot be put on a schedule. > > > > My skills are not for micro-optimizing codec code: I cannot help on these > > tasks. If I am allowed to analyze this myself, I would say that my skills > > lie in > > general organization: making sure that the right code gets called at the > > right > > time, finding more convenient ways of doing tasks, etc. > > > > I have several ideas for the project. Some are not directly related to > > multimedia at all; rather, the are invented for FFmpeg precisely, for > > FFmpeg's > > exact needs and ways of doing things. They relate to options and > > introspection, to inter-thread scheduling and asynchronous operation, to > > error reporting to the application, to handling of strings and > > serialization, to > > partial configurations of filters, to avoiding global state and allowing > > multiple > > instances, etc. > > > > I have shown the first steps of some of my ideas (AVWriter a few months > > ago, avrefcount_template.h more recently), and the outcome was rather > > disheartening and discouraging: "it's too complex" (of course: I am putting > > all > > the complexity in one place so that the rest of the code can be simple, if > > you > > look at just that part it looks complex!), "why don't you just" do thing the > > usual way? (because I am trying to find a better way than the usual way.) It > > seems my fellow developers do not look beyond the immediate strangeness > > of the proposal to see the promised benefits, but remember: strangeness is > > just lack of familiarity, it goes away fast all by itself, and all that > > remains are > > the benefits. > > > > These proposals would probably be better met if they were complete: if I > > proposed a patch series that eliminates escaping hell or gets non-blocking > > operation working all at once, it would be easier to get it accepted. But > > it is > > too big a task, especially with the rest of the code moving under my feet, > > with conflicts at each rebase. > > > > Lacking that, they require a little trust: trust enough to look, beyond the > > immediate strangeness, where I am going and to try to see what I want to > > achieve, and see that it is achievable. But for now, I have seen more doubt > > and dismissal than trust and enthusiasm. And the projects are too big, I am > > not prepared to fight an uphill battle to get acc
Re: [FFmpeg-devel] Project orientation
On 2020-07-04 07:43, Nicolas George wrote: [In the FFmpeg project,] [t]here is work in making highly-optimized decoders, this work is impressive and creative…. But as far as I can see, that is mostly all there is going on. The rest seems to be rather basic: fixing bugs…, implementing support for features that were decided and designed elsewhere. And it is not just that it is not happening: there is genuine opposition to creativity: things that are not already justified externally, things that do not look like well-known patterns, are met with scepticism and eventually rejected. At this point, we should ask ourselves: Is it what we want the project to be, or is it just what we have let it become? I do not think this evolution is the result of a deliberate choice. I think it is more the result of the stress of success and the stress of a fork. Success can stifle creativity, because creativity implies the risk of failure: the project has become advert to risk. It has evolved that way, but are we fine with it? I personally am not…. I am really happy to see this discussion starting up. It is good timing; some new governance committees are forming. They might be able to take it on. From my perspective as an outside observer, I think the project will benefit from you — the people who are at the heart of the project — having this discussion. For what purpose do all of you contribute to FFmpeg? What is the good you see FFmpeg doing? What is FFmpeg's purpose? What stands in the way of FFmpeg achieving its purpose, and of you achieving your own individual purpose? For me, several comments in the thread resonated. They have a theme: bringing in new participants: On 2020-07-04 08:37, James Almer wrote: …Another thing worth mentioning is a lack of new blood. Despite participating in GSoC for a long while, i can't name a student that stuck around after the fact. Mind, there are new devs that started contributing for other reasons, but perhaps not enough? On 2020-07-04 11:20, Soft Works wrote: …As a developer (without a well-known name) who wants to contribute a patch, things can be quite frustrating here. When that patch accidentally hits an area of one the very few who are caring and friendly - you're lucky. But otherwise, a patch will either be ignored or talked into infinity. I have a number of things to contribute, but after it didn’t work well with small things, I decided not to bother with the bigger ones. On 2020-07-05 11:42, Steinar H. Gunderson wrote: On Sun, Jul 05, 2020 at 08:13:25PM +0200, Manolis Stamatogiannakis wrote: As a fresh contributor, setting up git send-email was a hassle, but not an insurmountable obstacle. Speaking only for myself, having sent a single-digit number of patches to FFmpeg ever: Setting up git send-email was not a big turnoff. Having your patch being not responded to (whether being forgotten, not found interesting enough, or whatever other reason) was. To the extent you decide that you want new participants to join you on FFmpeg, I'm sorry to say, I think you have a problem. My own experience is that this project is more hostile to newcomers than many other free culture projects I know (e.g. Python, Thunderbird, Gnucash, Drupal, MusicBrainz, Wikipedia). It's not just your rudeness to each other on the email lists. It's not just the inattention to patches from new developers. It's also a simple lack of telling newcomers: welcome, we are glad you are here, we want your help, we will help you learn. Even more deeply, the culture of this project is focussed on checking in compileable code to the exclusion of other kinds of contribution: testing, documentation, support. Those contributions seem not welcome simply because they are not code, and code is all that matters. I suspect that you may find that some of the things that frustrate you are linked to work the project does not value. Repetitive questions on the ffmpeg-users list may in part result from the inadequate documentation, which doesn't tell users what they need to know. Feeling stretched thin over too much code without enough tooling might result from the too little of the right unit tests. And so on. I wish you success as you ask yourself these big questions. I hope it helps you have a more pleasant time in this project. I am confident it will result in a better FFmpeg. —Jim DeLaHunt, software engineer, Vancouver, Canada ___ 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] libavformat/hlsenc: Remove duplicate close of the output stream.
Andrey Semashev 于2020年7月5日周日 下午8:25写道: > > Ping? > > On 2020-07-01 17:59, Andrey Semashev wrote: > > The result of the first close attempt is ignored and may be lost. By > > removing > > it we ensure the close result code is properly analyzed. > > --- > > libavformat/hlsenc.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > > index 71fa3db060..88b58a1ba8 100644 > > --- a/libavformat/hlsenc.c > > +++ b/libavformat/hlsenc.c > > @@ -2631,7 +2631,6 @@ static int hls_write_trailer(struct AVFormatContext > > *s) > > goto failed; > > > > vs->size = range_length; > > -hlsenc_io_close(s, &vs->out, filename); > > ret = hlsenc_io_close(s, &vs->out, filename); > > if (ret < 0) { > > av_log(s, AV_LOG_WARNING, "upload segment failed, will retry > > with a new http session.\n"); > > > > ___ > 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". will apply Thanks Steven ___ 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/4] Supplement AVS3-P2/IEEE1857.10 video decoding via libuavs3d
Ping. Looking forward to more suggestions :-) At 2020-06-22 21:57:48, hwr...@126.com wrote: >From: hwren > >=== Version1 === >These patches are to supplement the third generation of Audio Video Coding >Standard, >part 2: video (AVS3-P2), aka IEEE1857.10, decoding support via libuavs3d >wrapper. > >The uAVS3d decoder could be found in https://github.com/uavs3/uavs3d >AVS3 sample streams could be found in https://github.com/uavs3/avs3stream > >=== Version 2 === >Fix conflict with CAVS streams. Considering that there is no direct version >flag in AVS, >AVS3 demuxer only supports raw streams in format <*.avs3>. > >Fix API function conflict. > >Thanks. > >hwren (4): > lavc: add AVS3 codec id and desc > lavc/avs3_paeser: add avs3 parser > lavf/avs3dec: add raw avs3 demuxer > lavc,doc: add libuavs3d video decoder wrapper > > Changelog| 1 + > configure| 4 + > doc/decoders.texi| 21 +++ > doc/general.texi | 8 ++ > libavcodec/Makefile | 2 + > libavcodec/allcodecs.c | 1 + > libavcodec/avs3_parser.c | 184 + > libavcodec/codec_desc.c | 7 + > libavcodec/codec_id.h| 1 + > libavcodec/libuavs3d.c | 283 +++ > libavcodec/parsers.c | 1 + > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/avs3dec.c| 75 +++ > 14 files changed, 590 insertions(+) > create mode 100644 libavcodec/avs3_parser.c > create mode 100644 libavcodec/libuavs3d.c > create mode 100644 libavformat/avs3dec.c > >-- >2.23.0.windows.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v06 2/5] fbtile helperRoutines cpu based framebuffer detiling
Jul 5, 2020, 23:47 by s...@jkqxz.net: > On 04/07/2020 14:17, hanishkvc wrote: > >> Add helper routines which can be used to detile tiled framebuffer >> layouts into a linear layout, using the cpu. >> >> Currently it supports Legacy Intel Tile-X, Legacy Intel Tile-Y and >> Newer Intel Tile-Yf tiled layouts. >> >> Currently supported pixel format is 32bit RGB. >> >> It also contains detile_generic logic, which can be easily configured >> to support different kinds of tiling layouts, at the expense of some >> processing speed, compared to developing a targeted detiling logic. >> --- >> libavutil/Makefile | 2 + >> libavutil/fbtile.c | 441 + >> libavutil/fbtile.h | 228 +++ >> 3 files changed, 671 insertions(+) >> create mode 100644 libavutil/fbtile.c >> create mode 100644 libavutil/fbtile.h >> > > I do think this is a reasonable thing to include in FFmpeg, but please think > about what you actually want as a public API here. > > Ideally you want to minimise the public API surface while providing something > clean and of general use. > > So: > - It should probably operate on whole frames - copying pieces of frames or > single planes doesn't have any obvious use. > - The pixel format, width, height and pitches will all need to be specified, > so AVFrames which already include that information are probably the right > structure to use. > - You want to specify a tiling mode for the source frame somehow. > - For the untile case the destination is linear, but a plausible use-case is > the opposite so we could include tiling mode for the destination as well. > - The tiling modes will need to be some sort of enum, since they are all > ad-hoc setups for particular hardware vendors. > - We can't reuse existing values like those from libdrm because we'd like it > to work everywhere it can and there is no intrinsic dependence on libdrm, so > it needs to be a new enum. > - Names for the tiling modes should be valid everywhere, so if they are > platform-dependent (like Intel X/Y tiling) then the platform will need to be > included in the name. > - Linear should be in the tiling mode enum, so that you don't need special > cases elsewhere. > - All the dispatch between different implementations can happen internally, > so it doesn't need to be exposed. > - Not everything will actually be implemented, so it will need to be able to > return an error indicating that a particular case is not available. > > Given that, I make the desired public API to be exactly one enum and one > function. It would look something like: > > enum AVTilingMode { > AV_TILING_MODE_LINEAR = 0, > AV_TILING_MODE_INTEL_GEN7_X, > AV_TILING_MODE_INTEL_GEN7_Y, > AV_TILING_MODE_INTEL_GEN9_X, > AV_TILING_MODE_INTEL_GEN9_Y, > AV_TILING_MODE_INTEL_YF, > }; > > int av_frame_copy_with_tiling(const AVFrame *dst, enum AVTilingMode > dst_tiling, > const AVFrame *src, enum AVTilingMode src_tiling); > > > Some other thoughts: > - Functions should all be static unless you are intentionally exposing them > as public API. > - Libraries must not include mutable globals. > - Note that av_log(NULL, ...) should never be called from a library unless > you really are in a global context. I think you probably just don't want to > include any user-facing logging at all. > - Look at the coding style guide, especially around naming and operator > spacing. > Yes, in general those are good guidelines for public APIs. But please, _don't_ in this case. Really. I've already said its not a good idea and I'll reject future patches that do simply because its a problem exclusive to hwcontext_drm and directly solvable there. ___ 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-cvslog] swscale: Add swscale input/output support for X2RGB10LE
> -Original Message- > From: ffmpeg-devel On Behalf Of > Michael Niedermayer > Sent: Monday, July 6, 2020 7:47 AM > To: ffmpeg-devel@ffmpeg.org > Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] swscale: Add swscale > input/output support for X2RGB10LE > > On Fri, Jun 12, 2020 at 04:57:30PM +, Fei Wang wrote: > > ffmpeg | branch: master | Fei Wang | Wed Apr 22 > > 13:23:02 2020 +0800| [c721b450141d6bbe1e977212a0bcb70118965c34] | > > committer: Lynne > > > > swscale: Add swscale input/output support for X2RGB10LE > > > > Signed-off-by: Fei Wang > > > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=c721b45014 > > > 1d6bbe1e977212a0bcb70118965c34 > > --- > > > > libswscale/input.c | 13 - > > libswscale/output.c | 14 ++ > > libswscale/utils.c | 1 + > > libswscale/yuv2rgb.c | 22 ++ > > tests/ref/fate/filter-pixfmts-copy | 1 + > > tests/ref/fate/filter-pixfmts-crop | 1 + > > tests/ref/fate/filter-pixfmts-field | 1 + > > tests/ref/fate/filter-pixfmts-fieldorder | 1 + > > tests/ref/fate/filter-pixfmts-hflip | 1 + > > tests/ref/fate/filter-pixfmts-il | 1 + > > tests/ref/fate/filter-pixfmts-null | 1 + > > tests/ref/fate/filter-pixfmts-pad| 1 + > > tests/ref/fate/filter-pixfmts-scale | 1 + > > tests/ref/fate/filter-pixfmts-transpose | 1 + > > tests/ref/fate/filter-pixfmts-vflip | 1 + > > 15 files changed, 60 insertions(+), 1 deletion(-) > > This fails on big endian (mips qemu) > > stuff like: Thanks, I will try to setup mips qumu to check this. Mostly like it shouldn't execute little endian case on big endian enviroment. @Andriy Gelman, I am not sure do we already have big endian test environment in patchwork fate test now? if not, we may consider to add it, so that we can catch the fail before patch merge. > > --- src/tests/ref/fate/filter-pixdesc-x2rgb10le 2020-07-05 19:59:34.522597589 > +0200 > +++ tests/data/fate/filter-pixdesc-x2rgb10le2020-07-06 00:35:27.242235755 > +0200 > @@ -1 +1 @@ > -pixdesc-x2rgb10le98d697ed4668daf535163d5e08c903bb > +pixdesc-x2rgb10le 1a608f8a86f297d6c560610927f91adb > Test filter-pixdesc-x2rgb10le failed. Look at tests/data/fate/filter-pixdesc- > x2rgb10le.err for details. > src/tests/Makefile:255: recipe for target 'fate-filter-pixdesc-x2rgb10le' > failed > make: *** [fate-filter-pixdesc-x2rgb10le] Error 1 > > > --- src/tests/ref/fate/filter-pixfmts-field 2020-07-05 19:59:34.522597589 > +0200 > +++ tests/data/fate/filter-pixfmts-field2020-07-06 00:37:10.830283380 > +0200 > @@ -80,7 +80,7 @@ > rgba64be23c8c0edaabe3eaec89ce69633fb0048 > rgba64ledfdba4de4a7cac9abf08852666c341d3 > uyvy422 1c49e44ab3f060e85fc4a3a9464f045e > -x2rgb10le a7a5dcdfe1d4b6bd71e40b01c735f144 > +x2rgb10le 05495c886b55fa5476374606f0148d68 > xyz12be d2fa69ec91d3ed862f2dac3f8e7a3437 > xyz12le 02bccd5e0b6824779a1f848b0ea3e3b5 > ya16be 40403b5277364777e0671da4d38e01ac > Test filter-pixfmts-field failed. Look at > tests/data/fate/filter-pixfmts-field.err for > details. > src/tests/Makefile:255: recipe for target 'fate-filter-pixfmts-field' failed > make: *** [fate-filter-pixfmts-field] Error 1 > > > --- src/tests/ref/fate/filter-pixfmts-hflip 2020-07-05 19:59:34.522597589 > +0200 > +++ tests/data/fate/filter-pixfmts-hflip2020-07-06 00:37:11.306283604 > +0200 > @@ -77,7 +77,7 @@ > rgba51961c723ea6707e0a410cd3f21f15d3 > rgba64bec910444019f4cfbf4d995227af55da8d > rgba64le0c810d8b3a6bca10321788e1cb145340 > -x2rgb10le 9f99dce306383daf25cd1542b2517fef > +x2rgb10le 583b0f0ae2642ddf24ab5abacba1bbc1 > xyz12be 25f90259ff8a226befdaec3dfe82996e > xyz12le 926c0791d59aaff61b2778e8ada3316d > ya16be d5b342355bdd9e3197e01b13b7c6301e > TESTvsynth2-dnxhd-1080i > Test filter-pixfmts-hflip failed. Look at > tests/data/fate/filter-pixfmts-hflip.err for > details. > src/tests/Makefile:255: recipe for target 'fate-filter-pixfmts-hflip' failed > make: *** [fate-filter-pixfmts-hflip] Error 1 > TESTvsynth2-dnxhd-1080i-10bit > TESTvsynth2-dnxhd-1080i-colr > TESTvsynth2-dnxhd-hr-lb-mov > TESTvsynth2-dnxhd-hr-sq-mov > --- src/tests/ref/fate/filter-pixfmts-copy 2020-07-05 19:59:34.522597589 > +0200 > +++ tests/data/fate/filter-pixfmts-copy 2020-07-06 00:37:11.622283754 > +++ +0200 > @@ -80,7 +80,7 @@ > rgba64beae2ae04b5efedca3505f47c4dd6ea6ea > rgba64leb91e1d77f799eb92241a2d2d28437b15 > uyvy422 3bcf3c80047592f2211fae3260b1b65d > -x2rgb10le b0a0c8056521beeaa3fea4985ca87176 > +x2rgb10le 6cb140adba429c301fecf464ed604506 > xyz12be a1ef56bf746d71f59669c28e48fc8450 > xyz12le
[FFmpeg-devel] Wow! SMPTE specs and journal articles, free (pandemic special)
The Society of Motion Picture & Television Engineers (SMPTE) is making available 16 standards documents and 12 journal articles for free download at https://www.smpte.org/free-standards-and-publications [1], until 21. July 2020. I think this is fantastic news, and might be of interest to many on this list. It deserves more than a passing mention. I suspect that FFmpeg already contains code related to some of these standards. Comparing that code to the official standards might be helpful. Also, there are probably references in comments to these standards, which would be even better if they uses specific section references to and consistent wording with the standards. Maybe some of these standards will be the basis of future FFmpeg features. And people who are interested in the the media operations FFmpeg does, might also be interested in the bigger pictures of the journal articles. My favourites on the list include ST 12-{1,2,3}:2014, previously known as SMPTE 12M, defining timecodes and and how to represent them as audio signals and as ancillary data. I have a client who would love to have better support for this in FFmpeg. I'm delighted to see that Limin Wang is adding[2] support for this data type. SMPTE says, "In these challenging times, access to learning and information is now more necessary than ever. And, as we continue to socially distance and safeguard ourselves and our loved ones, we want to share some of our SMPTE Standards and publications from our Motion Imaging Journal in hopes that you find these helpful to you and your work." The documents are provided in HTML form and as downloadable PDF files. The list is: SMPTE Standards • ST 2110-10:2017 - SMPTE Standard - Professional Media Over Managed IP Networks: System Timing and Definitions • ST 2110-20:2017 - SMPTE Standard - Professional Media Over Managed IP Networks: Uncompressed Active Video • ST 2110-21:2017 - SMPTE Standard - Professional Media Over Managed IP Networks: Traffic Shaping and Delivery Timing for Video • ST 2110-30:2017 - SMPTE Standard - Professional Media Over Managed IP Networks: PCM Digital Audio • ST 2110-40:2018 - SMPTE Standard - Professional Media Over Managed IP Networks: SMPTE ST 291-1 Ancillary Data • ST 2059-1:2015 - SMPTE Standard - Generation and Alignment of Interface Signals to the SMPTE Epoch • ST 2059-2:2015 - SMPTE Standard - SMPTE Profile for Use of IEEE-1588 Precision Time Protocol in Professional Broadcast Applications • ST 2067-2:2020 - SMPTE Standard - Interoperable Master Format — Core Constraints • ST 2067-3:2020 - SMPTE Standard - Interoperable Master Format — Composition Playlist • ST 2067-5:2020 - SMPTE Standard - Interoperable Master Format — Essence Component • ST 2067-21:2020 - SMPTE Standard - Interoperable Master Format — Application #2E • ST 12-1:2014 - SMPTE Standard - Time and Control Code • ST 12-2:2014 - SMPTE Standard - Transmission of Time Code in the Ancillary Data Space • ST 12-3:2016 - SMPTE Standard - Time Code for High Frame Rate Signals and Formatting in the Ancillary Data Space • ST 2022-7:2019 - SMPTE Standard - Seamless Protection Switching of RTP Datagrams • ST 2034-1:2017 - SMPTE Standard - Archive eXchange Format (AXF) - Part 1: Structure & Semantics Motion Imaging Journal • Time-Compensated Remote Production Over IP, Ed Calverley, June 2018 • Analyzing SMPTE ST 2110 Streams Using EBU’s Open-Source Software, Ievgen Kostiukevych, Willem Vermost, and Pedro Ferreira, May 2019 • Interoperable Workflows Through IMF Output Profile Lists, Arjun Ramamurthy and Raymond Yeung, August 2019 • AI in Production: Video Analysis and Machine Learning for Expanded Live Events Coverage, Craig Wright, Jack Allnutt, Rosie Campbell, Michael Evans, Ronan Forman, James Gibson, Stephen Jolly, Lianne Kerlin, Susan Lechelt, Graeme Phillipson, and Matthew Shotton, March 2020 • Editing in the Cloud, Ulrich Ening and Karsten Schragmann, March 2020 • Applying Automation to High-Quality/Low-Cost Orchestra Live Streaming, David R. Chalmers, April 2020 • Streaming Video Fundamentals, Jason Thibeault, April 2020 • How OTT Services Can Match the Quality of Broadcast, Thierry Fautier, April 2020 • Optimizing Mass-Scale Multiscreen Video Delivery, Yuriy Reznik, Xiangbo Li, Karl Lillevold, Robert Peck, Thom Shutt, and Peter Howard, April 2020 • How Independent are HDR, WCG, and HFR in Human Visual Perception and the Creative Process, S. T. McCarthy May-June 2016 • Cloud Transition Patterns for Media Enterprises, Shailendra Mathur, SMPTE 2017 ATC • Quality Control & Monitoring in OTT Workflow, Ramandeep Sandhu, SMPTE Australia 2017 Many thanks to Andreas Rheinhardt for mentioning[3] that these documents are available. [1] https://www.smpte.org/free-standards-and-publications [2] http://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/264781.html [3] http://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/265336.html Enjoy!
[FFmpeg-devel] [PATCH 1/2] avcodec/cbs: Remove unused function parameters
Several cbs-functions had an unused CodedBitstreamContext parameter. This commit removes these. Signed-off-by: Andreas Rheinhardt --- As one sees, removing the ctx parameter from some of these functions depends on removing it from others. But ff_cbs_alloc_unit_content is separate from all the others; so if we switched to Mark's ff_cbs_alloc_unit_content2 [1], one would not need to add this parameter back for any other function. [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/262042.html libavcodec/av1_frame_merge_bsf.c| 12 libavcodec/av1_frame_split_bsf.c| 10 +++--- libavcodec/av1_metadata_bsf.c | 14 - libavcodec/av1_parser.c | 6 ++-- libavcodec/cbs.c| 48 - libavcodec/cbs.h| 21 + libavcodec/cbs_av1.c| 4 +-- libavcodec/cbs_h264.h | 6 ++-- libavcodec/cbs_h2645.c | 40 libavcodec/cbs_jpeg.c | 10 +++--- libavcodec/cbs_mpeg2.c | 6 ++-- libavcodec/cbs_vp9.c| 6 ++-- libavcodec/filter_units_bsf.c | 8 ++--- libavcodec/h264_metadata_bsf.c | 24 +++ libavcodec/h264_redundant_pps_bsf.c | 8 ++--- libavcodec/h265_metadata_bsf.c | 13 libavcodec/mpeg2_metadata_bsf.c | 8 ++--- libavcodec/trace_headers_bsf.c | 6 ++-- libavcodec/vaapi_encode_h264.c | 13 libavcodec/vaapi_encode_h265.c | 13 libavcodec/vaapi_encode_mjpeg.c | 14 - libavcodec/vaapi_encode_mpeg2.c | 9 +++--- libavcodec/vp9_metadata_bsf.c | 4 +-- 23 files changed, 138 insertions(+), 165 deletions(-) diff --git a/libavcodec/av1_frame_merge_bsf.c b/libavcodec/av1_frame_merge_bsf.c index b5aa57e0ff..baea5596d7 100644 --- a/libavcodec/av1_frame_merge_bsf.c +++ b/libavcodec/av1_frame_merge_bsf.c @@ -34,8 +34,8 @@ static void av1_frame_merge_flush(AVBSFContext *bsf) { AV1FMergeContext *ctx = bsf->priv_data; -ff_cbs_fragment_reset(ctx->cbc, &ctx->frag[0]); -ff_cbs_fragment_reset(ctx->cbc, &ctx->frag[1]); +ff_cbs_fragment_reset(&ctx->frag[0]); +ff_cbs_fragment_reset(&ctx->frag[1]); av_packet_unref(ctx->in); av_packet_unref(ctx->pkt); } @@ -93,7 +93,7 @@ eof: ctx->idx = !ctx->idx; } else { for (i = 0; i < frag->nb_units; i++) { -err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type, +err = ff_cbs_insert_unit_content(tu, -1, frag->units[i].type, frag->units[i].content, frag->units[i].content_ref); if (err < 0) goto fail; @@ -108,7 +108,7 @@ eof: else av_packet_unref(in); -ff_cbs_fragment_reset(ctx->cbc, &ctx->frag[ctx->idx]); +ff_cbs_fragment_reset(&ctx->frag[ctx->idx]); fail: if (err < 0 && err != AVERROR(EAGAIN)) @@ -133,8 +133,8 @@ static void av1_frame_merge_close(AVBSFContext *bsf) { AV1FMergeContext *ctx = bsf->priv_data; -ff_cbs_fragment_free(ctx->cbc, &ctx->frag[0]); -ff_cbs_fragment_free(ctx->cbc, &ctx->frag[1]); +ff_cbs_fragment_free(&ctx->frag[0]); +ff_cbs_fragment_free(&ctx->frag[1]); av_packet_free(&ctx->in); av_packet_free(&ctx->pkt); ff_cbs_close(&ctx->cbc); diff --git a/libavcodec/av1_frame_split_bsf.c b/libavcodec/av1_frame_split_bsf.c index 87dfc83103..13bebe19f5 100644 --- a/libavcodec/av1_frame_split_bsf.c +++ b/libavcodec/av1_frame_split_bsf.c @@ -172,7 +172,7 @@ static int av1_frame_split_filter(AVBSFContext *ctx, AVPacket *out) if (s->cur_frame == s->nb_frames) { av_packet_unref(s->buffer_pkt); -ff_cbs_fragment_reset(s->cbc, td); +ff_cbs_fragment_reset(td); } return 0; @@ -187,7 +187,7 @@ fail: av_packet_unref(out); av_packet_unref(s->buffer_pkt); } -ff_cbs_fragment_reset(s->cbc, td); +ff_cbs_fragment_reset(td); return ret; } @@ -224,7 +224,7 @@ static int av1_frame_split_init(AVBSFContext *ctx) if (ret < 0) av_log(ctx, AV_LOG_WARNING, "Failed to parse extradata.\n"); -ff_cbs_fragment_reset(s->cbc, td); +ff_cbs_fragment_reset(td); return 0; } @@ -234,7 +234,7 @@ static void av1_frame_split_flush(AVBSFContext *ctx) AV1FSplitContext *s = ctx->priv_data; av_packet_unref(s->buffer_pkt); -ff_cbs_fragment_reset(s->cbc, &s->temporal_unit); +ff_cbs_fragment_reset(&s->temporal_unit); } static void av1_frame_split_close(AVBSFContext *ctx) @@ -242,7 +242,7 @@ static void av1_frame_split_close(AVBSFContext *ctx) AV1FSplitContext *s = ctx->priv_data; av_packet_free(&s->buffer_pkt); -ff_cbs_fragment_free(s->cbc, &s->temporal_unit); +ff_cbs_fragment_free(&s->temporal_unit); ff_cbs_close(&s->cbc); } diff --git a/libavcodec/av1_metadata_bsf.c b/libavcodec/av1_metadata_
[FFmpeg-devel] [PATCH 2/2] avcodec/h26[45]_metadata_bsf: Use separate contexts for reading/writing
Currently, both bsfs used the same CodedBitstreamContext for reading and writing; as a consequence, the state of the writer's context at the beginning of writing a fragment is exactly the state of the reader after having read the fragment; in particular, the writer might not have encountered one of its active parameter sets yet. This is not nice and may lead to invalid output even when the input is completely spec-compliant: Think of an access unit containing a primary coded picture referencing a PPS with id id (that is known from an earlier access unit/from extradata), then a new version of the PPS with id id and then a redundant coded picture that is also referencing the PPS with id id. This is spec-compliant, as the standard allows to overwrite a PPS with a different PPS in between coded pictures and not only at the beginning of an access unit. In this scenario, the reader would read the primary coded picture with the old PPS and the redundant coded picture with the new PPS (as it should); yet the writer would write both with the new PPS as extradata which might lead to errors or to invalid data being output without any error (e.g. if the two PPS differed in redundant_pic_cnt_present_flag). The above scenario does not directly translate to HEVC as long as one restricts oneself to input with nuh_layer_id == 0 only (as cbs_h265 does: it currently strips away any NAL unit with nuh_layer_id > 0 when decomposing); if one doesn't the same issue as above can happen. If one also allowed input packets to contain more than one access unit, issues like the above can happen even without redundant coded pictures/multiple layers. Therefore this commit uses separate contexts for reader and writer. Signed-off-by: Andreas Rheinhardt --- This is an alternative to James patch [1] which instead uses separate reader and writer parameter sets in the same CodedBitstreamContext. The diff would be bigger if it were not for the preceding patch (in this case one would have to choose one of the two contexts to add/delete units and as soon as one has to do this, one notices that none of the two choices make any sense). libavcodec/h264_metadata_bsf.c | 23 ++- libavcodec/h265_metadata_bsf.c | 23 ++- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c index 09aa1765fd..9f081a3879 100644 --- a/libavcodec/h264_metadata_bsf.c +++ b/libavcodec/h264_metadata_bsf.c @@ -49,7 +49,8 @@ enum { typedef struct H264MetadataContext { const AVClass *class; -CodedBitstreamContext *cbc; +CodedBitstreamContext *cbc_in; +CodedBitstreamContext *cbc_out; CodedBitstreamFragment access_unit; int done_first_au; @@ -289,7 +290,7 @@ static int h264_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt) if (!side_data_size) return 0; -err = ff_cbs_read(ctx->cbc, au, side_data, side_data_size); +err = ff_cbs_read(ctx->cbc_in, au, side_data, side_data_size); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to read extradata from packet side data.\n"); return err; @@ -303,7 +304,7 @@ static int h264_metadata_update_side_data(AVBSFContext *bsf, AVPacket *pkt) } } -err = ff_cbs_write_fragment_data(ctx->cbc, au); +err = ff_cbs_write_fragment_data(ctx->cbc_out, au); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to write extradata into packet side data.\n"); return err; @@ -334,7 +335,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt) if (err < 0) goto fail; -err = ff_cbs_read_packet(ctx->cbc, au, pkt); +err = ff_cbs_read_packet(ctx->cbc_in, au, pkt); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to read packet.\n"); goto fail; @@ -602,7 +603,7 @@ static int h264_metadata_filter(AVBSFContext *bsf, AVPacket *pkt) } } -err = ff_cbs_write_packet(ctx->cbc, pkt, au); +err = ff_cbs_write_packet(ctx->cbc_out, pkt, au); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to write packet.\n"); goto fail; @@ -626,12 +627,15 @@ static int h264_metadata_init(AVBSFContext *bsf) CodedBitstreamFragment *au = &ctx->access_unit; int err, i; -err = ff_cbs_init(&ctx->cbc, AV_CODEC_ID_H264, bsf); +err = ff_cbs_init(&ctx->cbc_in, AV_CODEC_ID_H264, bsf); +if (err < 0) +return err; +err = ff_cbs_init(&ctx->cbc_out, AV_CODEC_ID_H264, bsf); if (err < 0) return err; if (bsf->par_in->extradata) { -err = ff_cbs_read_extradata(ctx->cbc, au, bsf->par_in); +err = ff_cbs_read_extradata(ctx->cbc_in, au, bsf->par_in); if (err < 0) { av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n"); goto fail; @@ -645,7 +649,7 @@ static int h264_metadata_init(AVBSFContext *bsf) } } -err = ff_cbs_wri
Re: [FFmpeg-devel] Project orientation
> -Original Message- > From: ffmpeg-devel On Behalf Of > Andriy Gelman > Sent: Monday, July 6, 2020 2:31 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] Project orientation > > On Sun, 05. Jul 23:42, Soft Works wrote: > > > > > > > -Original Message- > > > From: ffmpeg-devel On Behalf Of > > > Michael Niedermayer > > > Sent: Monday, July 6, 2020 1:18 AM > > > To: FFmpeg development discussions and patches > > de...@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] Project orientation > > > > > > On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: > > > > ... A significant part of code reviews are code-style violations. > > > > This is really not something where humans should need to spend > > > > time for when reviewing a patch. > > > > > > you are correct but that is also the easy part of reviews. > > > Its not what makes reviews time consuming. > > > Rather while reviewing for technical stuff one notices maybe a > > > formating issue > > > > When then reviewer would not have to look for code style and could > > assume that this is all right, he would be free to focus on the actual > > things. > > And when it's only about code-style, a reviewer would not need to > > review a patch two times (original + corrected) for checking whether > > there were no other changes (imagine a multi-part patch). > > Also, the contributor would not have to wait two times for his patch > > to get reviewed. > > > > > Continuous integration processing could also list compiler warnings > > isolated to the patch and help a reviewer spot issues that he might > > have overlooked otherwise. > > This is done already... and checked for every single commit in the series. > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200701164348.41647 > -1-hanish...@gmail.com/ But still the "Michael-Machine" needed to respond manually: > This breaks build on non x86 > > src/libavfilter/vf_fbdetile.c:81:10: fatal error: x86intrin.h: No such file > or directory > #include > ^ > > [...] And everybody in the mailinglist had to read/read this patch even though it had an error. 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] Project orientation
On Sun, 05. Jul 23:42, Soft Works wrote: > > > > -Original Message- > > From: ffmpeg-devel On Behalf Of > > Michael Niedermayer > > Sent: Monday, July 6, 2020 1:18 AM > > To: FFmpeg development discussions and patches > de...@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] Project orientation > > > > On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: > > > ... A significant part of code reviews are code-style violations. This > > > is really not something where humans should need to spend time for > > > when reviewing a patch. > > > > you are correct but that is also the easy part of reviews. > > Its not what makes reviews time consuming. > > Rather while reviewing for technical stuff one notices maybe a formating > > issue > > When then reviewer would not have to look for code style and could > assume that this is all right, he would be free to focus on the actual things. > And when it's only about code-style, a reviewer would not need to > review a patch two times (original + corrected) for checking whether > there were no other changes (imagine a multi-part patch). > Also, the contributor would not have to wait two times for his patch > to get reviewed. > > Continuous integration processing could also list compiler warnings > isolated to the patch and help a reviewer spot issues that he might > have overlooked otherwise. This is done already... and checked for every single commit in the series. https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200701164348.41647-1-hanish...@gmail.com/ -- 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] [FFmpeg-cvslog] swscale: Add swscale input/output support for X2RGB10LE
On Fri, Jun 12, 2020 at 04:57:30PM +, Fei Wang wrote: > ffmpeg | branch: master | Fei Wang | Wed Apr 22 > 13:23:02 2020 +0800| [c721b450141d6bbe1e977212a0bcb70118965c34] | committer: > Lynne > > swscale: Add swscale input/output support for X2RGB10LE > > Signed-off-by: Fei Wang > > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=c721b450141d6bbe1e977212a0bcb70118965c34 > --- > > libswscale/input.c | 13 - > libswscale/output.c | 14 ++ > libswscale/utils.c | 1 + > libswscale/yuv2rgb.c | 22 ++ > tests/ref/fate/filter-pixfmts-copy | 1 + > tests/ref/fate/filter-pixfmts-crop | 1 + > tests/ref/fate/filter-pixfmts-field | 1 + > tests/ref/fate/filter-pixfmts-fieldorder | 1 + > tests/ref/fate/filter-pixfmts-hflip | 1 + > tests/ref/fate/filter-pixfmts-il | 1 + > tests/ref/fate/filter-pixfmts-null | 1 + > tests/ref/fate/filter-pixfmts-pad| 1 + > tests/ref/fate/filter-pixfmts-scale | 1 + > tests/ref/fate/filter-pixfmts-transpose | 1 + > tests/ref/fate/filter-pixfmts-vflip | 1 + > 15 files changed, 60 insertions(+), 1 deletion(-) This fails on big endian (mips qemu) stuff like: --- src/tests/ref/fate/filter-pixdesc-x2rgb10le 2020-07-05 19:59:34.522597589 +0200 +++ tests/data/fate/filter-pixdesc-x2rgb10le2020-07-06 00:35:27.242235755 +0200 @@ -1 +1 @@ -pixdesc-x2rgb10le98d697ed4668daf535163d5e08c903bb +pixdesc-x2rgb10le 1a608f8a86f297d6c560610927f91adb Test filter-pixdesc-x2rgb10le failed. Look at tests/data/fate/filter-pixdesc-x2rgb10le.err for details. src/tests/Makefile:255: recipe for target 'fate-filter-pixdesc-x2rgb10le' failed make: *** [fate-filter-pixdesc-x2rgb10le] Error 1 --- src/tests/ref/fate/filter-pixfmts-field 2020-07-05 19:59:34.522597589 +0200 +++ tests/data/fate/filter-pixfmts-field2020-07-06 00:37:10.830283380 +0200 @@ -80,7 +80,7 @@ rgba64be23c8c0edaabe3eaec89ce69633fb0048 rgba64ledfdba4de4a7cac9abf08852666c341d3 uyvy422 1c49e44ab3f060e85fc4a3a9464f045e -x2rgb10le a7a5dcdfe1d4b6bd71e40b01c735f144 +x2rgb10le 05495c886b55fa5476374606f0148d68 xyz12be d2fa69ec91d3ed862f2dac3f8e7a3437 xyz12le 02bccd5e0b6824779a1f848b0ea3e3b5 ya16be 40403b5277364777e0671da4d38e01ac Test filter-pixfmts-field failed. Look at tests/data/fate/filter-pixfmts-field.err for details. src/tests/Makefile:255: recipe for target 'fate-filter-pixfmts-field' failed make: *** [fate-filter-pixfmts-field] Error 1 --- src/tests/ref/fate/filter-pixfmts-hflip 2020-07-05 19:59:34.522597589 +0200 +++ tests/data/fate/filter-pixfmts-hflip2020-07-06 00:37:11.306283604 +0200 @@ -77,7 +77,7 @@ rgba51961c723ea6707e0a410cd3f21f15d3 rgba64bec910444019f4cfbf4d995227af55da8d rgba64le0c810d8b3a6bca10321788e1cb145340 -x2rgb10le 9f99dce306383daf25cd1542b2517fef +x2rgb10le 583b0f0ae2642ddf24ab5abacba1bbc1 xyz12be 25f90259ff8a226befdaec3dfe82996e xyz12le 926c0791d59aaff61b2778e8ada3316d ya16be d5b342355bdd9e3197e01b13b7c6301e TESTvsynth2-dnxhd-1080i Test filter-pixfmts-hflip failed. Look at tests/data/fate/filter-pixfmts-hflip.err for details. src/tests/Makefile:255: recipe for target 'fate-filter-pixfmts-hflip' failed make: *** [fate-filter-pixfmts-hflip] Error 1 TESTvsynth2-dnxhd-1080i-10bit TESTvsynth2-dnxhd-1080i-colr TESTvsynth2-dnxhd-hr-lb-mov TESTvsynth2-dnxhd-hr-sq-mov --- src/tests/ref/fate/filter-pixfmts-copy 2020-07-05 19:59:34.522597589 +0200 +++ tests/data/fate/filter-pixfmts-copy 2020-07-06 00:37:11.622283754 +0200 @@ -80,7 +80,7 @@ rgba64beae2ae04b5efedca3505f47c4dd6ea6ea rgba64leb91e1d77f799eb92241a2d2d28437b15 uyvy422 3bcf3c80047592f2211fae3260b1b65d -x2rgb10le b0a0c8056521beeaa3fea4985ca87176 +x2rgb10le 6cb140adba429c301fecf464ed604506 xyz12be a1ef56bf746d71f59669c28e48fc8450 xyz12le 831ff03c1ba4ef19374686f16a064d8c ya16be 37c07787e544f900c87b853253bfc8dd Test filter-pixfmts-copy failed. Look at tests/data/fate/filter-pixfmts-copy.err for details. src/tests/Makefile:255: recipe for target 'fate-filter-pixfmts-copy' failed make: *** [fate-filter-pixfmts-copy] Error 1 TESTvsynth2-dnxhd-hr-hq-mov [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If the United States is serious about tackling the national security threats related to an insecure 5G network, it needs to rethink the extent to which it values corporate profits and government espionage over security.-Bruce Schneier signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-deve
Re: [FFmpeg-devel] Project orientation
> -Original Message- > From: ffmpeg-devel On Behalf Of > Michael Niedermayer > Sent: Monday, July 6, 2020 1:18 AM > To: FFmpeg development discussions and patches de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] Project orientation > > On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: > > ... A significant part of code reviews are code-style violations. This > > is really not something where humans should need to spend time for > > when reviewing a patch. > > you are correct but that is also the easy part of reviews. > Its not what makes reviews time consuming. > Rather while reviewing for technical stuff one notices maybe a formating > issue When then reviewer would not have to look for code style and could assume that this is all right, he would be free to focus on the actual things. And when it's only about code-style, a reviewer would not need to review a patch two times (original + corrected) for checking whether there were no other changes (imagine a multi-part patch). Also, the contributor would not have to wait two times for his patch to get reviewed. Continuous integration processing could also list compiler warnings isolated to the patch and help a reviewer spot issues that he might have overlooked otherwise. > > Neither should it be required that Michael responds manually each time > > by stating "breaks FATE". > > We already have fully automatic build and fate tests for submitted patches its > done by patchwork which should send a private mail to the submitter of a > patch affected. And still you're sending those e-mails.. > Still not every environment is the same, and it passing on the patchwork box > doesnt mean it builds on my mingw or mips environments for example. Such things could be set up easily, either for free or for little money... ;-) 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] ABI break in 4.3
On Sun, Jul 05, 2020 at 11:48:14PM +0200, Jan Engelhardt wrote: > > On Sunday 2020-07-05 23:17, Michael Niedermayer wrote: > > > >so its all doable in theory, but at that point the question arises, can we > >maybe generate this automatically from the APIchanges file if we decide to > >do that. > >APIchanges should list all added functions and the version numbers. > > APIchanges would have to become more "machine-readable". yes > Like, for example, > https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist machines are not that stupid. The existing format is fine, it just has to be consistent just consider git grep '[Aa]dd [a-z0-9_]*()' doc/APIchanges that probably catches most added functions, the previous hash from each gives you the git version to match it to. Sure it wont work for 100% but the 5% this fails for can be just edited to be consistent so it works too and it keeps the format enjoyable for humans to read and write [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Take away the freedom of one citizen and you will be jailed, take away the freedom of all citizens and you will be congratulated by your peers in Parliament. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On 06.07.2020 01:18, Michael Niedermayer wrote: On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: ... A significant part of code reviews are code-style violations. This is really not something where humans should need to spend time for when reviewing a patch. you are correct but that is also the easy part of reviews. Its not what makes reviews time consuming. Rather while reviewing for technical stuff one notices maybe a formating issue Neither should it be required that Michael responds manually each time by stating "breaks FATE". We already have fully automatic build and fate tests for submitted patches its done by patchwork which should send a private mail to the submitter of a patch affected. Still not every environment is the same, and it passing on the patchwork box doesnt mean it builds on my mingw or mips environments for example. It also very frequently fails to properly handle large and complex series, with some of the patches getting new versions. Also can't properly handle testing patches for stuff it cannot build, because it's for a different arch or needs a very specific library. But that would equally affect any kind of Gitlab/Github Workflow we could set up. ___ 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] Project orientation
On Sun, Jul 05, 2020 at 09:56:02PM +, Soft Works wrote: > ... A significant part of code reviews are code-style violations. This is > really not something where humans should need to spend time for when > reviewing a patch. you are correct but that is also the easy part of reviews. Its not what makes reviews time consuming. Rather while reviewing for technical stuff one notices maybe a formating issue > Neither should it be required that Michael responds manually each time > by stating "breaks FATE". We already have fully automatic build and fate tests for submitted patches its done by patchwork which should send a private mail to the submitter of a patch affected. Still not every environment is the same, and it passing on the patchwork box doesnt mean it builds on my mingw or mips environments for example. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB "I am not trying to be anyone's saviour, I'm trying to think about the future and not be sad" - Elon Musk signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/7] avcodec/cbs: Check for overflow when reading
On 09/12/2019 22:25, Andreas Rheinhardt wrote: While CBS itself uses size_t for sizes, it relies on other APIs that use int for their sizes; in particular, AVBuffer uses int for their size parameters and so does GetBitContext with their number of bits. While CBS aims to be a safe API, the checks it employed were not sufficient to prevent overflows: E.g. if the size of a unit was > UINT_MAX / 8, 8 * said size may be truncated to a positive integer before being passed to init_get_bits() in which case its return value would not indicate an error. These checks have been improved to really capture these kinds of errors; furthermore, although the sizes are still size_t, they are now de-facto restricted to 0..INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE. Signed-off-by: Andreas Rheinhardt --- The check in cbs_insert_unit() can currently not be triggered, because av_malloc_array makes sure that it doesn't allocate more than INT_MAX; so the allocation will fail long before we get even close to INT_MAX. av1 and H.26x have not been changed, because their split functions already check the size (in case of H.264 and H.265 this happens in ff_h2645_packet_split()). It would btw be possible to open the GetBitContext generically. The read_unit function would then get the already opened GetBitContext as a parameter. libavcodec/cbs.c | 6 ++ libavcodec/cbs_jpeg.c | 2 +- libavcodec/cbs_mpeg2.c | 2 +- libavcodec/cbs_vp9.c | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index 0badb192d9..805049404b 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -206,6 +206,9 @@ static int cbs_fill_fragment_data(CodedBitstreamContext *ctx, { av_assert0(!frag->data && !frag->data_ref); +if (size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE) +return AVERROR(ERANGE); For this and the following patch I wonder if it would be nicer to pick a sensible upper bounds for fragments (something like 2^30B total in at most 2^20 units?), name them (CBS_MAX_DATA_SIZE, CBS_MAX_UNITS?) and then use those in all checks? No real case should get anywhere near these bounds, and indeed anything which even gets close was likely crafted by a malicious adversary specifically to do so. + frag->data_ref = av_buffer_alloc(size + AV_INPUT_BUFFER_PADDING_SIZE); if (!frag->data_ref) @@ -693,6 +696,9 @@ static int cbs_insert_unit(CodedBitstreamContext *ctx, memmove(units + position + 1, units + position, (frag->nb_units - position) * sizeof(*units)); } else { +if (frag->nb_units == INT_MAX) +return AVERROR(ERANGE); + units = av_malloc_array(frag->nb_units + 1, sizeof(*units)); if (!units) return AVERROR(ENOMEM); diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c index b189cbd9b7..2bb6c8d18c 100644 --- a/libavcodec/cbs_jpeg.c +++ b/libavcodec/cbs_jpeg.c @@ -246,7 +246,7 @@ static int cbs_jpeg_read_unit(CodedBitstreamContext *ctx, GetBitContext gbc; int err; -err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); +err = init_get_bits8(&gbc, unit->data, unit->data_size); if (err < 0) return err; diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index 13d871cc89..255f033734 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -227,7 +227,7 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx, GetBitContext gbc; int err; -err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); +err = init_get_bits8(&gbc, unit->data, unit->data_size); if (err < 0) return err; diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c index 42e4dcf5ac..f6cfaa3b36 100644 --- a/libavcodec/cbs_vp9.c +++ b/libavcodec/cbs_vp9.c @@ -488,7 +488,7 @@ static int cbs_vp9_read_unit(CodedBitstreamContext *ctx, GetBitContext gbc; int err, pos; -err = init_get_bits(&gbc, unit->data, 8 * unit->data_size); +err = init_get_bits8(&gbc, unit->data, unit->data_size); if (err < 0) return err; There are more of these init_get_bits(..., 8 * something) hanging around. Perhaps change all of them as one patch, even if there isn't any danger of overflow? Thanks, - Mark ___ 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 v06 2/5] fbtile helperRoutines cpu based framebuffer detiling
On 04/07/2020 14:17, hanishkvc wrote: Add helper routines which can be used to detile tiled framebuffer layouts into a linear layout, using the cpu. Currently it supports Legacy Intel Tile-X, Legacy Intel Tile-Y and Newer Intel Tile-Yf tiled layouts. Currently supported pixel format is 32bit RGB. It also contains detile_generic logic, which can be easily configured to support different kinds of tiling layouts, at the expense of some processing speed, compared to developing a targeted detiling logic. --- libavutil/Makefile | 2 + libavutil/fbtile.c | 441 + libavutil/fbtile.h | 228 +++ 3 files changed, 671 insertions(+) create mode 100644 libavutil/fbtile.c create mode 100644 libavutil/fbtile.h I do think this is a reasonable thing to include in FFmpeg, but please think about what you actually want as a public API here. Ideally you want to minimise the public API surface while providing something clean and of general use. So: - It should probably operate on whole frames - copying pieces of frames or single planes doesn't have any obvious use. - The pixel format, width, height and pitches will all need to be specified, so AVFrames which already include that information are probably the right structure to use. - You want to specify a tiling mode for the source frame somehow. - For the untile case the destination is linear, but a plausible use-case is the opposite so we could include tiling mode for the destination as well. - The tiling modes will need to be some sort of enum, since they are all ad-hoc setups for particular hardware vendors. - We can't reuse existing values like those from libdrm because we'd like it to work everywhere it can and there is no intrinsic dependence on libdrm, so it needs to be a new enum. - Names for the tiling modes should be valid everywhere, so if they are platform-dependent (like Intel X/Y tiling) then the platform will need to be included in the name. - Linear should be in the tiling mode enum, so that you don't need special cases elsewhere. - All the dispatch between different implementations can happen internally, so it doesn't need to be exposed. - Not everything will actually be implemented, so it will need to be able to return an error indicating that a particular case is not available. Given that, I make the desired public API to be exactly one enum and one function. It would look something like: enum AVTilingMode { AV_TILING_MODE_LINEAR = 0, AV_TILING_MODE_INTEL_GEN7_X, AV_TILING_MODE_INTEL_GEN7_Y, AV_TILING_MODE_INTEL_GEN9_X, AV_TILING_MODE_INTEL_GEN9_Y, AV_TILING_MODE_INTEL_YF, }; int av_frame_copy_with_tiling(const AVFrame *dst, enum AVTilingMode dst_tiling, const AVFrame *src, enum AVTilingMode src_tiling); Some other thoughts: - Functions should all be static unless you are intentionally exposing them as public API. - Libraries must not include mutable globals. - Note that av_log(NULL, ...) should never be called from a library unless you really are in a global context. I think you probably just don't want to include any user-facing logging at all. - Look at the coding style guide, especially around naming and operator spacing. Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] libavcodec/jpeg2000dec: Enhance pix fmt selection
On Sun, Jul 05, 2020 at 12:33:08AM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > This patch assigns default pix format values when > a match does not take place. > --- > libavcodec/jpeg2000dec.c | 12 > 1 file changed, 12 insertions(+) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/jpeg2000dec.c: Enable image offsets
On Sun, Jul 05, 2020 at 12:33:09AM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > This patch enables support for image offsets. > --- > libavcodec/jpeg2000dec.c | 4 > 1 file changed, 4 deletions(-) will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avcodec/tiff: Check frame parameters before blit for DNG
On Sat, Jul 04, 2020 at 02:45:52PM +0200, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: > 23888/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_TIFF_fuzzer-6021365974171648.fuzz > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/tiff.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/mjpegdec: Limit bayer to single plane outputting format
On Sat, Jul 04, 2020 at 02:45:51PM +0200, Michael Niedermayer wrote: > This reduces the number of paths reachable with DNG and should > improve security > > Signed-off-by: Michael Niedermayer > --- > libavcodec/mjpegdec.c | 5 + > 1 file changed, 5 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
July 5, 2020 11:43 PM, "Manolis Stamatogiannakis" wrote: > On Sun, 5 Jul 2020 at 19:01, Kieran Kunhya wrote: > >> For new contributors git send-email is annoying. For people wanting to >> push, the .mbox format is annoying, Gmail doesn't support it any more. >> And you can't get new contributors to start using CLI based email >> clients or run their own mail server, that's not going to happen. > > As a fresh contributor, setting up git send-email was a hassle, but > not an insurmountable obstacle. > > Though I'd argue that git send-email is a bigger liability for regular > developers. A lot of developers seem to be using Gmail, which means they > need to have "less secure apps" enabled at the time they use git > send-email. So they are forced to choose between security and convenience. > I.e. having to temporarily enable "less secure apps" every time they > submit, or accept the risk of weakened security for their account. Although not exactly convenient, Gmail now has a feature called app passwords that allows you to create specialised passwords for things like e-mail clients. You however have to enable 2 factor authentication for this. https://support.google.com/accounts/answer/185833?hl=en >> A solution like Gitlab is the only way forward. It has worked well for >> dav1d, it can run regression tests on all platforms for all commits: >> https://code.videolan.org/videolan/dav1d >> >> Merges are done with one push of a button. Yes, the branch sprawl is >> not great but it's better than now. >> It has inline patch reviews which are nice. > > FWIW, I'd add that branch-based PRs as implemented with GitHub greatly > reduce the turnaround for receiving feedback and addressing the raised > issues. > All you have to do is force-push on your PR branch, and the PR is cleanly > updated. > This is in contrast with having to use git send-email, where the emails > containing the stale patches will continue to litter patchwork and your > mailboxes. > > Also, IIRC, when you update a branch, the regression tests for all pending > PRs on the branch are automatically rerun. > This should help address the conflicts in the pending PRs sooner rather > than later. > > Not sure what the status of GitLab is wrt to these features, but I'd expect > it to not be very far behind. > > Best regards, > Manolis Regards, Anamitra ___ 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] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input
On Sun, Jul 5, 2020 at 5:56 PM Zhong Li wrote: > > > I can't see the benefit to use MSDK framerate conversion function. Is > > > it a good idea to drop it and use ffmpeg native fps filter instead? > > > > Reconsidering this, leaving the filter buggy doesn't seem to be > comfortable to me, > > hence IMHO it'll be better to have this bug fixed. > My suggestion would be just delete MSDK framerate conversion instead > of patch it. > Will send a patch if nobody against. > should be ok when comes with proper description, as comes with price of performance regards Max ___ 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] Project orientation
> -Original Message- > From: ffmpeg-devel On Behalf Of > Jean-Baptiste Kempf > Sent: Sunday, July 5, 2020 11:48 PM > To: ffmpeg-devel > Subject: Re: [FFmpeg-devel] Project orientation > > On Sun, Jul 5, 2020, at 22:50, Michael Niedermayer wrote: > > > Having your > > > patch being not responded to (whether being forgotten, not found > > > interesting enough, or whatever other reason) was. > > > > At least for me the reason to not review a patch is often simply time. > > But i agree the amount of not reviewed patches is a problem. > > Yes. > > > How can we solve this ? > > With tools and organization. > > Today, any patchset that has several patches (like the 36 ones for VP9 of the > other day) or multiple revisions gets everyone a ton of emails. When you are > at the revision 3 or 7, you completely lost track, unless you know exactly > what to look for. Seeing the difference from the 2 revisions is very hard. So > either you spend a lot of time understanding and re-reading or you drop it. > So, either inefficiency for the reviewer, and therefore less reviews, or > forgotten patches. > > Today the patchwork has 64 pages of patches... > > Noone human can know all that is happening. > > So either you split the mailing list and patches per library (different for > libavcodec, format), with submaintainers for each library, basically, like the > Linux Kernel does. > Or you move to github/gitlab which gives you by default a tracking of the MR > that got merged or not, where you can see quickly the differences between > 2 revisions of a patchset and where you can use tags to mark the merge > requests, where the CI is automatic, so you don't have to check manually > that the code compiles or that FATE is not broken, etc... +1 for this. A significant part of code reviews are code-style violations. This is really not something where humans should need to spend time for when reviewing a patch. Neither should it be required that Michael responds manually each time by stating "breaks FATE". Also, this would allow review comments stay connected to the commented code parts forever. When in the future somebody wonders why something has been done in a certain way, it will be easy to find that discussion. 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 submission questions
Thanks for the tips Andriy. To make it easier for future new contributors I took the time to update developers.texi with the information I got from this thread, and revamp the chapter about submitting patches. Feedback is most welcome. Best regards, Manolis On Sun, 5 Jul 2020 at 20:56, Andriy Gelman wrote: > On Sun, 05. Jul 20:34, Manolis Stamatogiannakis wrote: > > On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong > > wrote: > > > > > > > > You can use `git format-patch -v -n` to get patches like [PATCH > > > v2 1/20]. See git-format-patch documentation for more details. > > > > > > > > That didn't quite work. > > > > I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org > > HEAD~2..HEAD" and "git send-email -2". > > > > But the -v argument was dropped and the end results on patchwork were > again: > > [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for > > subtitles/ass filters. > > [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter > > options. > > > > git send-email -v2 HEAD~ # works for me > > git shows are draft of the email before sending. Check that the subject > line > contains the version number (or you can send the patch to yourself) > > For patchwork, you can create an account and set your old patches as > 'Superseded'. > I also periodically run a script that sets Superseded and Accepted labels. > > -- > 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". ___ 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] ABI break in 4.3
On Sunday 2020-07-05 23:17, Michael Niedermayer wrote: > >so its all doable in theory, but at that point the question arises, can we >maybe generate this automatically from the APIchanges file if we decide to >do that. >APIchanges should list all added functions and the version numbers. APIchanges would have to become more "machine-readable". Like, for example, https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist ___ 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] Project orientation
On Sun, Jul 5, 2020, at 22:50, Michael Niedermayer wrote: > > Having your > > patch being not responded to (whether being forgotten, not found interesting > > enough, or whatever other reason) was. > > At least for me the reason to not review a patch is often simply > time. > But i agree the amount of not reviewed patches is a problem. Yes. > How can we solve this ? With tools and organization. Today, any patchset that has several patches (like the 36 ones for VP9 of the other day) or multiple revisions gets everyone a ton of emails. When you are at the revision 3 or 7, you completely lost track, unless you know exactly what to look for. Seeing the difference from the 2 revisions is very hard. So either you spend a lot of time understanding and re-reading or you drop it. So, either inefficiency for the reviewer, and therefore less reviews, or forgotten patches. Today the patchwork has 64 pages of patches... Noone human can know all that is happening. So either you split the mailing list and patches per library (different for libavcodec, format), with submaintainers for each library, basically, like the Linux Kernel does. Or you move to github/gitlab which gives you by default a tracking of the MR that got merged or not, where you can see quickly the differences between 2 revisions of a patchset and where you can use tags to mark the merge requests, where the CI is automatic, so you don't have to check manually that the code compiles or that FATE is not broken, etc... -- Jean-Baptiste Kempf - President +33 672 704 734 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 2/3] doc/developer.texi: Restructured "Submitting patches" section.
- Main text split to two sections. - Detailed checklist for new codecs or formats demoted to section. - Detailed checklist for patch submission demoted to section. Signed-off-by: Manolis Stamatogiannakis --- doc/developer.texi | 64 +++--- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/doc/developer.texi b/doc/developer.texi index dec27cb509..6d4f6afcf9 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -457,31 +457,49 @@ Finally, keep in mind the immortal words of Bill and Ted, @anchor{Submitting patches} @chapter Submitting patches -First, read the @ref{Coding Rules} above if you did not yet, in particular +@anchor{patch guidelines} +@section Guidelines for preparing a patch + +The @strong{absolute minimum} you have to do before submitting a patch are the +following: + +@enumerate +@item Carefully read the @ref{Coding Rules} above if you did not yet, in particular the rules regarding patch submission. -When you submit your patch, please use @code{git format-patch} or -@code{git send-email}. We cannot read other diffs :-). +@item Make sure your commit messages accurately describe the changes made +(e.g. 'replaces lrint by lrintf') and why these changes are made (e.g. +'*BSD isn't C99 compliant and has no lrint()'). -Also please do not submit a patch which contains several unrelated changes. -Split it into separate, self-contained pieces. This does not mean splitting -file by file. Instead, make the patch as small as possible while still -keeping it as a logical unit that contains an individual change, even -if it spans multiple files. This makes reviewing your patches much easier -for us and greatly increases your chances of getting your patch applied. +@item Make sure you use @code{git format-patch} or @code{git send-email} to prepare +your patch. We cannot read other diffs :-). + +@item Run the @ref{Regression tests, regression tests} before submitting a patch +in order to verify it does not cause unexpected problems. -Use the patcheck tool of FFmpeg to check your patch. -The tool is located in the tools directory. +@item If you send your patches with an external email client +(i.e. not @code{git send-email}), make sure to send each patch as a separate +email. Do not attach several patches to the same email! -Run the @ref{Regression tests} before submitting a patch in order to verify -it does not cause unexpected problems. +@item Do not submit a patch which contains several unrelated changes. +@end enumerate + +Additionally, it is also important that the commits comprising a patch +are logically self-contained. I.e. each commit should be as small as +possible while still containing a meaningful individual change. +Commits spanning multiple files are perfectly fine, as long as the +commit can be seen as a single logical unit. -It also helps quite a bit if you tell us what the patch does (for example -'replaces lrint by lrintf'), and why (for example '*BSD isn't C99 compliant -and has no lrint()') +Following these guidelines makes reviewing your patches much easier +for us and greatly increases your chances of getting your patch applied. +To further reduce the chance that you will need to revise your patch, +it is also recommended to go through the detailed +@ref{patch submission checklist, patch} and +@ref{new codec format checklist, new codec or format} +checklists. -Also please if you send several patches, send each patch as a separate mail, -do not attach several unrelated patches to the same mail. +@anchor{patch submission process} +@section Patch submission and revision process Patches should be posted to the @uref{https://lists.ffmpeg.org/mailman/listinfo/ffmpeg-devel, ffmpeg-devel} @@ -511,7 +529,8 @@ Additionally, it is recommended to register for a This will allow you to mark previous version of your patches as "Superseded", and reduce the chance of someone spending time to review a stale patch. -@chapter New codecs or formats checklist +@anchor{new codec format checklist} +@section New codecs or formats checklist @enumerate @item @@ -563,7 +582,8 @@ Did you make sure it compiles standalone, i.e. with @end enumerate -@chapter Patch submission checklist +@anchor{patch submission checklist} +@section Patch submission checklist @enumerate @item @@ -592,6 +612,10 @@ of @dfn{sign-off}. @item Did you provide a clear git commit log message? +@item +Did you use the @code{patcheck} tool of FFmpeg to check your patch +for common issues? E.g. @code{tools/patcheck *.patch}. + @item Is the patch against latest FFmpeg git master branch? -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 3/3] doc/developer.texi: Swapped patch checklist and new codec/format checklist.
Adding a new codec/format should be more rare, so it makes sense to come after the detailed patch submission checklist. Signed-off-by: Manolis Stamatogiannakis --- doc/developer.texi | 105 ++--- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/doc/developer.texi b/doc/developer.texi index 6d4f6afcf9..cecb10fed1 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -529,59 +529,6 @@ Additionally, it is recommended to register for a This will allow you to mark previous version of your patches as "Superseded", and reduce the chance of someone spending time to review a stale patch. -@anchor{new codec format checklist} -@section New codecs or formats checklist - -@enumerate -@item -Did you use av_cold for codec initialization and close functions? - -@item -Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or -AVInputFormat/AVOutputFormat struct? - -@item -Did you bump the minor version number (and reset the micro version -number) in @file{libavcodec/version.h} or @file{libavformat/version.h}? - -@item -Did you register it in @file{allcodecs.c} or @file{allformats.c}? - -@item -Did you add the AVCodecID to @file{avcodec.h}? -When adding new codec IDs, also add an entry to the codec descriptor -list in @file{libavcodec/codec_desc.c}. - -@item -If it has a FourCC, did you add it to @file{libavformat/riff.c}, -even if it is only a decoder? - -@item -Did you add a rule to compile the appropriate files in the Makefile? -Remember to do this even if you're just adding a format to a file that is -already being compiled by some other rule, like a raw demuxer. - -@item -Did you add an entry to the table of supported formats or codecs in -@file{doc/general.texi}? - -@item -Did you add an entry in the Changelog? - -@item -If it depends on a parser or a library, did you add that dependency in -configure? - -@item -Did you @code{git add} the appropriate files before committing? - -@item -Did you make sure it compiles standalone, i.e. with -@code{configure --disable-everything --enable-decoder=foo} -(or @code{--enable-demuxer} or whatever your component is)? -@end enumerate - - @anchor{patch submission checklist} @section Patch submission checklist @@ -708,6 +655,58 @@ Test your code with valgrind and or Address Sanitizer to ensure it's free of leaks, out of array accesses, etc. @end enumerate +@anchor{new codec format checklist} +@section New codecs or formats checklist + +@enumerate +@item +Did you use av_cold for codec initialization and close functions? + +@item +Did you add a long_name under NULL_IF_CONFIG_SMALL to the AVCodec or +AVInputFormat/AVOutputFormat struct? + +@item +Did you bump the minor version number (and reset the micro version +number) in @file{libavcodec/version.h} or @file{libavformat/version.h}? + +@item +Did you register it in @file{allcodecs.c} or @file{allformats.c}? + +@item +Did you add the AVCodecID to @file{avcodec.h}? +When adding new codec IDs, also add an entry to the codec descriptor +list in @file{libavcodec/codec_desc.c}. + +@item +If it has a FourCC, did you add it to @file{libavformat/riff.c}, +even if it is only a decoder? + +@item +Did you add a rule to compile the appropriate files in the Makefile? +Remember to do this even if you're just adding a format to a file that is +already being compiled by some other rule, like a raw demuxer. + +@item +Did you add an entry to the table of supported formats or codecs in +@file{doc/general.texi}? + +@item +Did you add an entry in the Changelog? + +@item +If it depends on a parser or a library, did you add that dependency in +configure? + +@item +Did you @code{git add} the appropriate files before committing? + +@item +Did you make sure it compiles standalone, i.e. with +@code{configure --disable-everything --enable-decoder=foo} +(or @code{--enable-demuxer} or whatever your component is)? +@end enumerate + @chapter Patch review process All patches posted to ffmpeg-devel will be reviewed, unless they contain a -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v1 1/3] doc/developer.texi: Improvements in "Submitting patches" section.
The section has been expanded to outline how to manage patch revisions. --- doc/developer.texi | 37 ++--- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/doc/developer.texi b/doc/developer.texi index b33cab0fc7..dec27cb509 100644 --- a/doc/developer.texi +++ b/doc/developer.texi @@ -491,18 +491,25 @@ as base64-encoded attachments, so your patch is not trashed during transmission. Also ensure the correct mime type is used (text/x-diff or text/x-patch or at least text/plain) and that only one patch is inline or attached per mail. -You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show up, its mime type -likely was wrong. +You can check the most recently received patches on +@url{https://patchwork.ffmpeg.org, patchwork}. +If your patch does not show up, its mime type likely was wrong. -Your patch will be reviewed on the mailing list. You will likely be asked +Your patch will be reviewed on the mailing list. Give us a few days to react. +But if some time passes without reaction, send a reminder by email. +Your patch should eventually be dealt with. However, you will likely be asked to make some changes and are expected to send in an improved version that incorporates the requests from the review. This process may go through several iterations. Once your patch is deemed good enough, some developer will pick it up and commit it to the official FFmpeg tree. -Give us a few days to react. But if some time passes without reaction, -send a reminder by email. Your patch should eventually be dealt with. - +When preparing an updated version of you patch, make sure you increment +its @emph{roll-counter}. This is achieved by adding a @code{-v } argument +to @code{git format-patch}/@code{git send-email} commands. +Additionally, it is recommended to register for a +@uref{https://patchwork.ffmpeg.org, patchwork account}. +This will allow you to mark previous version of your patches as "Superseded", +and reduce the chance of someone spending time to review a stale patch. @chapter New codecs or formats checklist @@ -563,7 +570,19 @@ Did you make sure it compiles standalone, i.e. with Does @code{make fate} pass with the patch applied? @item -Was the patch generated with git format-patch or send-email? +Was the patch generated with @code{git format-patch} or @code{git send-email}? + +@item +If you are submitting a revised patch, did you increment the roll-counter +with @code{-v }? + +@item +If you are submitting a revised patch, did you marked previous versions of +the patch as "Superseded" on @uref{https://patchwork.ffmpeg.org/, patchwork}? + +@item +Are you subscribed to ffmpeg-devel? +(the list is subscribers only due to spam) @item Did you sign-off your patch? (@code{git commit -s}) @@ -576,10 +595,6 @@ Did you provide a clear git commit log message? @item Is the patch against latest FFmpeg git master branch? -@item -Are you subscribed to ffmpeg-devel? -(the list is subscribers only due to spam) - @item Have you checked that the changes are minimal, so that the same cannot be achieved with a smaller patch and/or simpler final code? -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, Jul 5, 2020, at 18:25, Marton Balint wrote: > > People aren't using it because people don't use MPEG-2. > > There is no maintenance to be done on a format that's 25 years old, do > > you want me to randomly change cosmetics to make you feel happy? > > If you'd get off your high horse for once, that would make me feel happy. This kind of tone is unwelcome. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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] Project orientation
On Sun, Jul 5, 2020, at 14:28, Marton Balint wrote: > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... You are missing the point here: Lego Racers demuxer is in the scope of "play everything under the sun" that the FFmpeg project is, while AMQP/ZMQ is not. The issue is not usefulness, but correctly defining the scope of the project (aka the orienttation). If you don't, you will integrate a webserver, an email sender, a full webRTC stack with p2p and a coffee controller. Some codecs and formats are not useful, sure, but they are in the scope, and there is little debate about it. But custom protocols or complex filters that bring huge dependencies are very debatable, and I doubt they are the consensus. Also in this example, noone is telling them to fork, just to use the API to register their custom protocol. -- Jean-Baptiste Kempf - President +33 672 704 734 ___ 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] Project orientation
On Sun, Jul 5, 2020 at 6:01 PM Kieran Kunhya wrote: > > Going back to the original point in hand. > Many patches aren't getting reviewed and pushed any more. > > In part this is because in 2020 whether we like it or not mailing > lists are not the way to do Git based development. > The kernel is the exception to the rule, as Linus says it has a whole > load of grey-bearded system maintainers who are paid full time to work > on it. > > For new contributors git send-email is annoying. For people wanting to > push, the .mbox format is annoying, Gmail doesn't support it any more. > And you can't get new contributors to start using CLI based email > clients or run their own mail server, that's not going to happen. > > A solution like Gitlab is the only way forward. It has worked well for > dav1d, it can run regression tests on all platforms for all commits: > https://code.videolan.org/videolan/dav1d > > Merges are done with one push of a button. Yes, the branch sprawl is > not great but it's better than now. > It has inline patch reviews which are nice. > > Whether we like it or not web interfaces are the way 95% of the world > does Git and we have to move with the times. Not my intention to top post but gmail hides quoted text. Forgot to add that git send-email is quite complex to setup now without your own mail server. This also restricts our ability to add new developers. Kieran ___ 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] Project orientation
On Sun, Jul 05, 2020 at 10:50:14PM +0200, Michael Niedermayer wrote: > At least for me the reason to not review a patch is often simply > time. > But i agree the amount of not reviewed patches is a problem. > > [...] > > The second thing is more reviews. > That can happen by > * More reviewers > * More reviews per reviewer > * Less work per review I think this depends on whether the problem is time at any given instant, or over a longer period of time. If the problem is “people don't have time right now”, what is needed is a way to make sure patches are not forgotten until the point where someone has time (so, effectively more reviews per reviewer). If the problem is that reviewers are generally overworked, one needs more reviewers or less work per review, as you say. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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] ABI break in 4.3
On Sun, Jul 05, 2020 at 05:08:51PM +0200, Tomas Härdin wrote: > sön 2020-07-05 klockan 14:10 +0200 skrev Jan Engelhardt: > > On Sunday 2020-07-05 13:39, Tomas Härdin wrote: > > > > > Downgrading to a .so file with a lower minor version number than > > > > > the program is built against can never be expected to work. Else > > > > > we couldn't add new functions without a major bump. > > > > > > > > It requires the use ELF symbol versions -- which ffmpeg fails to > > > > do properly.[...] > > > > > > This is a fair point. I didn't actually know the loader can do stuff > > > like this, sounds super handy. How hard would it be to get that going? > > > > Change libavcodec.v to > > > > LIBAVCODEC_58 { > > global: > > av_foo; > > av_bar; > > av_whatever_else_there_is;... > > local: > > *; > > }; > > LIBAVCODEC_58.91 { > > global: > > avpriv_mpeg4audio_get_config2; > > } LIBAVCODEC_58; > > LIBAVCODEC_58.123 { /* future example */ > > global: > > avblahblah; > > } LIBAVCODEC_58.91; > > > > Repeat likewise for other .v files. The file is no longer a template. The > > automatic substitution of "_MAJOR" by the build system needs to cease. > > Version > > numbers in the file are supposed to be fixed (among the set of all .so files > > that share a SONAME). > > Interesting. This also makes what's changed between versions more > explicit. Can this be checked by machine as well? Like having a post- > receive hook that makes sure the .v files are consistent, or maybe have > FATE check it somehow. its possible to do some sanity checks at the git level that would catch some issues. (for example you can detect changed version numbers and reject if that doesnt also update APIchanges and then parse the change to APIchanges and the change to the .v file and check if the same functions and versions are listed.) At the fate level it should be easier to detect the actual functions in the object files and compare this to the .v files so its all doable in theory, but at that point the question arises, can we maybe generate this automatically from the APIchanges file if we decide to do that. APIchanges should list all added functions and the version numbers. > > This probably needs to be passed through the technical committee. But I > don't think it'll break the ABI like Timo suggests, if we bump minor at > the same time. the technical committee is for disagreement arbitraration. ATM i dont think we have any disagreement [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I am the wisest man alive, for I know one thing, and that is that I know nothing. -- Socrates signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
Going back to the original point in hand. Many patches aren't getting reviewed and pushed any more. In part this is because in 2020 whether we like it or not mailing lists are not the way to do Git based development. The kernel is the exception to the rule, as Linus says it has a whole load of grey-bearded system maintainers who are paid full time to work on it. For new contributors git send-email is annoying. For people wanting to push, the .mbox format is annoying, Gmail doesn't support it any more. And you can't get new contributors to start using CLI based email clients or run their own mail server, that's not going to happen. A solution like Gitlab is the only way forward. It has worked well for dav1d, it can run regression tests on all platforms for all commits: https://code.videolan.org/videolan/dav1d Merges are done with one push of a button. Yes, the branch sprawl is not great but it's better than now. It has inline patch reviews which are nice. Whether we like it or not web interfaces are the way 95% of the world does Git and we have to move with the times. Kieran On Sun, Jul 5, 2020 at 5:38 PM Kieran Kunhya wrote: > > > > People aren't using it because people don't use MPEG-2. > > > There is no maintenance to be done on a format that's 25 years old, do > > > you want me to randomly change cosmetics to make you feel happy? > > > > If you'd get off your high horse for once, that would make me feel happy. > > Please let me know what "maintenance" you'd like me to do on x262? > > > I don't think I told anybody to implement the missing features for free. > > But I do believe that payed or free development has a higher chance of > > happening if existing code is already available in a popular > > package/project. And yes, I believe that some "maintenance burden" should > > be accepted by the base project to give more chance to further > > advancement, payed or free. > > It's not your pejorative to say that x264 developers have to accept > and maintain a merge back of x262. > Also this is quite complex now owing the combined 8/10-bit single > binary. Furthermore any change to H.264 they would now have to test > and maintain for MPEG-2. > You seem to imply that this work is quite simple which is easy for you > to say when you are the one not doing it. > Do you plan to do this work? > > Kieran ___ 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] Project orientation
On Sun, Jul 5, 2020 at 9:10 PM Marton Balint wrote: > I don't know enough about x262/x264 to do this with reasonable amount of > work. Do you think there is a chance of this happening if I post a bounty > or get a sponsorship? x264 is an H.264/AVC encoder and as such an MPEG-2 encoder is not in scope to be included. Just because a use case exists doesn't mean it's a good idea to accept every feature under the sun upstream. Having a separate fork for that is perfectly reasonable. ___ 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] Project orientation
On Sun, Jul 05, 2020 at 08:42:13PM +0200, Steinar H. Gunderson wrote: > On Sun, Jul 05, 2020 at 08:13:25PM +0200, Manolis Stamatogiannakis wrote: > > As a fresh contributor, setting up git send-email was a hassle, but > > not an insurmountable obstacle. > > Speaking only for myself, having sent a single-digit number of patches > to FFmpeg ever: Setting up git send-email was not a big turnoff. > Having your > patch being not responded to (whether being forgotten, not found interesting > enough, or whatever other reason) was. At least for me the reason to not review a patch is often simply time. But i agree the amount of not reviewed patches is a problem. How can we solve this ? From a mathematical point of view either more reviews must happen or fewer patches have to be sent. Fewer patches would only make sense if they are replaced by selfreview and direct commits. This could reduce the load on reviewers. We did this in the past and we had fewer non reviewed patches back then. I do not know what peoples oppinion is about this but there has been opposition to this long ago when it was commonly done. The second thing is more reviews. That can happen by * More reviewers * More reviews per reviewer * Less work per review To Achieve this, we could try to * attract more developers doing reviews, i have generally suggested contributors to help review other peoples patches. Maybe i should take a step back and ask developers to ask developers to do this instead. It is a way out of this problem * make people have a burning desire to review patches. I understand this would work very well but iam not sure how to achieve this * pay developers to do reviews, i think we do not yet have the funds for this as reviews take alot of time and thus this would not be cheap Also to make reviews less work * Code documentation should be improved * Testcases in fate should be mandatory for newly added "parts", this allows easy testing of changes and allows filtering out some bugs automatically Some simple suggestion * If you submmit a patch and its reviewed by someone, please pick some other patch by someone in an area you reasonably understand and review it (If everyone does this we have no lack of reviewers anymore) Anyway i dont think the unreviewed patches amount is a unsolveable problem Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are best at talking, realize last or never when they are wrong. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v06 4/5] hwdownload detile framebuffer, if requested by user
Hi Lynne, On Sun, 5 Jul, 2020, 00:59 Lynne, wrote: > Jul 4, 2020, 14:17 by hanish...@gmail.com: > > > Added logic to support detiling of framebuffer. > > > > By default this is disabled. Only if requested by the user, the > > logic will be triggered. > > > > It uses the fbtile helper routines to do the detiling. Currently > > 32bit RGB pixel format based framebuffers are supported. > > > > If the underlying hardware context provides linear layouts, then > > nothing is done. Only if the underlying hardware context generates > > tiled layout, then user can use this to detile, where possible. > > > > ./ffmpeg -f kmsgrab -i - -vf hwdownload=1,format=bgr0 out.mp4 > > > > Not acceptable for the reasons stated in the hwcontext patch. > Besides, allowing users to detile non-tiled images is a really, _really_ > bad idea. > As already responded, in case of the hwcontext patch, I take care of avoiding the frame copy, so that it's efficient, where possible. While this patch is different, as mentioned in this patch note. The idea here is to provide a fall back option for detiling such that it's independent of the underlying hwcontext, in case if such a situation arises in future. And Or Parallely it allows the user to force a detile, if they so desire explicitly. As this is not automatic, but has to be triggered by the user, It doesn't add any overhead by default, but gives the flexibility in case the user so desires. I do understand that there is a fundamental difference of opinion wrt whether to give this additional flexibility to user or not between my thinking and yours. I wanted to just show that it can be done in a simple and relatively clean way without overhead, when not desired. However It's finally your+any additional ffmpeg teams call. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
> > People aren't using it because people don't use MPEG-2. > > There is no maintenance to be done on a format that's 25 years old, do > > you want me to randomly change cosmetics to make you feel happy? > > If you'd get off your high horse for once, that would make me feel happy. Please let me know what "maintenance" you'd like me to do on x262? > I don't think I told anybody to implement the missing features for free. > But I do believe that payed or free development has a higher chance of > happening if existing code is already available in a popular > package/project. And yes, I believe that some "maintenance burden" should > be accepted by the base project to give more chance to further > advancement, payed or free. It's not your pejorative to say that x264 developers have to accept and maintain a merge back of x262. Also this is quite complex now owing the combined 8/10-bit single binary. Furthermore any change to H.264 they would now have to test and maintain for MPEG-2. You seem to imply that this work is quite simple which is easy for you to say when you are the one not doing it. Do you plan to do this work? Kieran ___ 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 v06 3/5] hwcontext_drm detile non linear layout, if possible
Hi Lynne, Am confused? Please look at this patch again. I directly detile from mmaped hardware framebuffer into specified output buffer. Only if the tile layout format is not supported, it falls back to the original frame copy. I am assuming you have misread the patch. On Sun, 5 Jul, 2020, 00:58 Lynne, wrote: > Jul 4, 2020, 14:17 by hanish...@gmail.com: > > > If the framebuffer is a tiled layout, use the fbtile helper routines > > to try and detile it into linear layout, if supported by fbtile. > > > > It uses the format_modifier associated with the framebuffer to decide > > whether to apply detiling or not and inturn which specific detiling > > to apply. > > > > If user is using kmsgrab, they will have to use -format_modifer option > > of kmsgrab to force a specific detile logic, in case they dont want to > > use the original format_modifier related detiling. Or they could even > > use -format_modifier 0 to make hwcontext_drm bypass this detiling. > > --- > > Changelog | 1 + > > libavutil/hwcontext_drm.c | 32 ++-- > > 2 files changed, 31 insertions(+), 2 deletions(-) > > > > diff --git a/Changelog b/Changelog > > index 3881587caa..b6a4ad1b34 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to > youngest within each release, > > releases are sorted from youngest to oldest. > > > > version : > > +- hwcontext_drm detiles non linear layouts, if possible > > - kmsgrab GetFB2 format_modifier, if user doesnt specify > > - AudioToolbox output device > > - MacCaption demuxer > > diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c > > index 32cbde82eb..bd74b3f13d 100644 > > --- a/libavutil/hwcontext_drm.c > > +++ b/libavutil/hwcontext_drm.c > > @@ -21,6 +21,7 @@ > > #include > > > > #include > > +#include > > #include > > > > #include "avassert.h" > > @@ -28,6 +29,7 @@ > > #include "hwcontext_drm.h" > > #include "hwcontext_internal.h" > > #include "imgutils.h" > > +#include "fbtile.h" > > > > > > static void drm_device_free(AVHWDeviceContext *hwdev) > > @@ -185,6 +187,32 @@ static int > drm_transfer_get_formats(AVHWFramesContext *ctx, > > return 0; > > } > > > > +// Can be overridden during compiling, if required. > > +#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER > > +#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1 > > +#endif > > > > This is also not acceptable, I'm afraid. We don't change behavior with > compile-time checks, > and those are really not the correct way to do it. > I get why you want to be able to dump tiled video but please understand, > we can't accept > this as-is. > Instead of looking for ways to make hacks part of the code base why not > look into improving > the performance of detiling? There's so much more to do than copying a > frame and detiling it. > You could copy and detile in-place, or you can avoid copying entirely by > just directly detiling > out of place to the destination frame. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v06 1/5] KMSGrab: getfb2 format_modifier if user doesnt specify
Hi Don't apply this patch, I will try and use ioctl directly instead of using xf86's GetFB2. Based on the reported compile issue with initialising global const for the fbtile patch when using older versions of gcc, I was checking the same using Ubuntu 16.04 setup, and realised that the older xf86drmMode.h in it doesn't provide GetFB2. On Sun, 5 Jul, 2020, 00:51 Lynne, wrote: > Jul 4, 2020, 14:17 by hanish...@gmail.com: > > > If user doesnt specify a format_modifier explicitly, then use GetFB2 > > to identify the format_modifier of the framebuffer being grabbed. > > --- > > Changelog | 1 + > > libavdevice/kmsgrab.c | 22 +- > > 2 files changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/Changelog b/Changelog > > index a60e7d2eb8..3881587caa 100644 > > --- a/Changelog > > +++ b/Changelog > > @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to > youngest within each release, > > releases are sorted from youngest to oldest. > > > > version : > > +- kmsgrab GetFB2 format_modifier, if user doesnt specify > > - AudioToolbox output device > > - MacCaption demuxer > > > > diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c > > index d0de774871..10ed707e60 100644 > > --- a/libavdevice/kmsgrab.c > > +++ b/libavdevice/kmsgrab.c > > @@ -239,6 +239,7 @@ static av_cold int > kmsgrab_read_header(AVFormatContext *avctx) > > drmModePlaneRes *plane_res = NULL; > > drmModePlane *plane = NULL; > > drmModeFB *fb = NULL; > > +drmModeFB2 *fb2 = NULL; > > AVStream *stream; > > int err, i; > > > > @@ -364,6 +365,23 @@ static av_cold int > kmsgrab_read_header(AVFormatContext *avctx) > > goto fail; > > } > > > > +fb2 = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id); > > +if (!fb2) { > > +err = errno; > > +av_log(avctx, AV_LOG_ERROR, "Failed to get " > > + "framebuffer2 %"PRIu32": %s.\n", > > + plane->fb_id, strerror(err)); > > +err = AVERROR(err); > > +goto fail; > > +} > > + > > +av_log(avctx, AV_LOG_INFO, "Template framebuffer2 is %"PRIu32": " > > + "%"PRIu32"x%"PRIu32", pixel_format: 0x%"PRIx32", > format_modifier: 0x%"PRIx64".\n", > > + fb2->fb_id, fb2->width, fb2->height, fb2->pixel_format, > fb2->modifier); > > + > > +if (ctx->drm_format_modifier == DRM_FORMAT_MOD_INVALID) > > +ctx->drm_format_modifier = fb2->modifier; > > + > > stream = avformat_new_stream(avctx, NULL); > > if (!stream) { > > err = AVERROR(ENOMEM); > > @@ -408,6 +426,8 @@ fail: > > drmModeFreePlane(plane); > > if (fb) > > drmModeFreeFB(fb); > > +if (fb2) > > +drmModeFreeFB2(fb2); > > > > return err; > > } > > @@ -433,7 +453,7 @@ static const AVOption options[] = { > > { .i64 = AV_PIX_FMT_BGR0 }, 0, UINT32_MAX, FLAGS }, > > { "format_modifier", "DRM format modifier for framebuffer", > > OFFSET(drm_format_modifier), AV_OPT_TYPE_INT64, > > - { .i64 = DRM_FORMAT_MOD_NONE }, 0, INT64_MAX, FLAGS }, > > + { .i64 = DRM_FORMAT_MOD_INVALID}, 0, INT64_MAX, FLAGS }, > > { "crtc_id", "CRTC ID to define capture source", > > OFFSET(source_crtc), AV_OPT_TYPE_INT64, > > { .i64 = 0 }, 0, UINT32_MAX, FLAGS }, > > -- > > 2.25.1 > > > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-requ...@ffmpeg.org> with subject "unsubscribe". > > > > This one looks fine to me, but Mark Thompson should check this one too. > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
> x264 is practically feature complete, but x262 still miss some things, > like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, > but it is still not packaged in e.g. Ubuntu, so users are stuck with the > - in some ways - inferior mpeg2 encoder of ffmpeg. > > The point I am trying to make is that you and some other people made a > fast and modern mpeg2 encoder, in some ways superior to existing open > source alternatives, but very few people is using it because it was not > merged to a bigger/more popular project like x264 or ffmpeg. So it > receives no mainteance, no distribution support, no user base and > ultimately no further development. Or at least that is how I see it. People aren't using it because people don't use MPEG-2. There is no maintenance to be done on a format that's 25 years old, do you want me to randomly change cosmetics to make you feel happy? I know of people using it 24/7 for many years and have had no issues. Have you opened bug reports on x264-devel about the issues you see? MPEG-2 4:2:2 interlaced users are infinitesimally small and professional users. If you want your pet feature to work, lo and behold you have to work on it or pay someone to work on it. If you want people to work on it out of nowhere then you are part of the Free Rider Problem: https://en.wikipedia.org/wiki/Free-rider_problem x264 is heavily used but the changes these days are very minor in spite of all these things. So your argument is incorrect. Kieran ___ 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] ABI break in 4.3
On 7/5/2020 3:59 PM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 19:21, James Almer wrote: >>> 8 files changed, 1163 insertions(+), 44 deletions(-) >> >> The list of functions is incomplete. After a quick look, you're for >> example missing av_map_videotoolbox_format_* in libavutil.[..] >> This is another issue that i don't know if you considered, and that's >> the fact currently some symbols may not present depending on configure >> time options, build environment, and target system. Videotoolbox is of >> course not available for you unless you're targeting OSX. > > An alternative to building an initial list from `nm` is to > visually work through the function declarations in .h files. > This also requires knowing which of those get installed to /usr/include > and which constitute private headers. The list of installed headers is $(HEADERS) in each of the project's library specific Makefiles. > I am sure you are more capable of knowing which ones are > which; I have only worked with ffmpeg source for a day. My intention was to state that ultimately it may not be a simple task to get a fixed, non build time generated .ver file for this purpose. The fact some symbols (at least currently) can be present or not may play against it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: People aren't using it because people don't use MPEG-2. > There is no maintenance to be done on a format that's 25 years old, do > you want me to randomly change cosmetics to make you feel happy? If you'd get off your high horse for once, that would make me feel happy. Please let me know what "maintenance" you'd like me to do on x262? I don't want you to do anything. But for example some code changes need to be merged back from x264 if somebody wants to make x262 compile using the latest ffmpeg. I don't think I told anybody to implement the missing features for free. But I do believe that payed or free development has a higher chance of happening if existing code is already available in a popular package/project. And yes, I believe that some "maintenance burden" should be accepted by the base project to give more chance to further advancement, payed or free. It's not your pejorative to say that x264 developers have to accept and maintain a merge back of x262. I am not saying they have to. I am saying that x262 would have benefited from it and the maintenance burden as I estimate it would not have been unsurmountable for x264. Also this is quite complex now owing the combined 8/10-bit single binary. Furthermore any change to H.264 they would now have to test and maintain for MPEG-2. You seem to imply that this work is quite simple which is easy for you to say when you are the one not doing it. I honestly don't know how much work it is. I assumed it is not too much, especially if somebody is familiar with the codebase. Now that the trees have diverged, obviously it has become harder. But dealing with the changes is usually easier if the feature is in-tree already. Do you plan to do this work? I don't know enough about x262/x264 to do this with reasonable amount of work. Do you think there is a chance of this happening if I post a bounty or get a sponsorship? Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On Sunday 2020-07-05 19:21, James Almer wrote: >> 8 files changed, 1163 insertions(+), 44 deletions(-) > >The list of functions is incomplete. After a quick look, you're for >example missing av_map_videotoolbox_format_* in libavutil.[..] >This is another issue that i don't know if you considered, and that's >the fact currently some symbols may not present depending on configure >time options, build environment, and target system. Videotoolbox is of >course not available for you unless you're targeting OSX. An alternative to building an initial list from `nm` is to visually work through the function declarations in .h files. This also requires knowing which of those get installed to /usr/include and which constitute private headers. I am sure you are more capable of knowing which ones are which; I have only worked with ffmpeg source for a day. ___ 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 submission questions
On Sun, 05. Jul 20:34, Manolis Stamatogiannakis wrote: > On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong > wrote: > > > > > You can use `git format-patch -v -n` to get patches like [PATCH > > v2 1/20]. See git-format-patch documentation for more details. > > > > > That didn't quite work. > > I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org > HEAD~2..HEAD" and "git send-email -2". > > But the -v argument was dropped and the end results on patchwork were again: > [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for > subtitles/ass filters. > [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter > options. > git send-email -v2 HEAD~ # works for me git shows are draft of the email before sending. Check that the subject line contains the version number (or you can send the patch to yourself) For patchwork, you can create an account and set your old patches as 'Superseded'. I also periodically run a script that sets Superseded and Accepted labels. -- 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] Project orientation
On Sun, Jul 05, 2020 at 08:13:25PM +0200, Manolis Stamatogiannakis wrote: > As a fresh contributor, setting up git send-email was a hassle, but > not an insurmountable obstacle. Speaking only for myself, having sent a single-digit number of patches to FFmpeg ever: Setting up git send-email was not a big turnoff. Having your patch being not responded to (whether being forgotten, not found interesting enough, or whatever other reason) was. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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 submission questions
On Sun, 5 Jul 2020 at 15:48, Hongcheng Zhong wrote: > > You can use `git format-patch -v -n` to get patches like [PATCH > v2 1/20]. See git-format-patch documentation for more details. > > That didn't quite work. I used "git format-patch -s -n -v3 --to ffmpeg-devel@ffmpeg.org HEAD~2..HEAD" and "git send-email -2". But the -v argument was dropped and the end results on patchwork were again: [FFmpeg-devel,2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters. [FFmpeg-devel,1/2] avfilter/vf_subtitles: Reorganized subtitles filter options. Manolis ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video. This avoids having to rewrite the subtitle file in order to display subtitles correctly when input is seeked (-ss). Also handy for minor subtitle timing corrections without rewriting the subtitles file. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 11 libavfilter/vf_subtitles.c | 55 +- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index d2b8feb14b..a8af563551 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options: @item filename, f Set the filename of the subtitle file to read. It must be specified. +@item shift +Shift subtitles timings by the specified amount. + @item original_size Specify the size of the original video, the video for which the ASS file was composed. For the syntax of this option, check the @@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. + +@item shift +Shift subtitles timings by the specified amount. @end table Additional options for @ref{subtitles} filter: @@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt} appear in 80% transparent blue subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF' @end example +To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use: +@example +subtitles=filename=sub.srt:shift='-20\:20' +@end example + @section super2xsai Scale the input by 2x and smooth using the Super2xSaI (Scale and diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c index 1bd42391e0..af1bc9cd18 100644 --- a/libavfilter/vf_subtitles.c +++ b/libavfilter/vf_subtitles.c @@ -52,6 +52,7 @@ typedef struct AssContext { char *filename; char *fontsdir; char *charenc; +int64_t shift; char *force_style; int stream_index; int alpha; @@ -66,11 +67,12 @@ typedef struct AssContext { #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM #define COMMON_OPTIONS \ -{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ -{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ +{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"shift", "shift subtitles timing", OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN, INT64_MAX, FLAGS }, \ /* libass supports a log level ranging from 0 to 7 */ static const int ass_libavfilter_log_level_map[] = { @@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx) return AVERROR(EINVAL); } +if (ass->shift != 0) { +ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1, 1000)); +av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n", ass->shift/1000.0); +} + ass->library = ass_library_init(); if (!ass->library) { av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n"); @@ -227,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass); static av_cold int init_ass(AVFilterContext *ctx) { +int eid, nskip; +ASS_Event *event; AssContext *ass = ctx->priv; int ret = init(ctx); @@ -243,6 +252,25 @@ static av_cold int init_ass(AVFilterContext *ctx)
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters. Rather than mentioning for each option whether it is common or not, the options are now displayed in two separate tables. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index ad2448acb2..d2b8feb14b 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that corresponds to option of @item planes @end table +@anchor{ass} @section ass Same as the @ref{subtitles} filter, except that it doesn't require libavcodec @@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to configure FFmpeg with libavformat to convert the passed subtitles file to ASS (Advanced Substation Alpha) subtitles format. -The filter accepts the following options: +Common @ref{subtitles}/@ref{ass} filter options: @table @option @item filename, f @@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. +@end table +Additional options for @ref{subtitles} filter: + +@table @option @item charenc -Set subtitles input character encoding. @code{subtitles} filter only. Only -useful if not UTF-8. +Set subtitles input character encoding. Only useful if not UTF-8. @item stream_index, si -Set subtitles stream index. @code{subtitles} filter only. +Set subtitles stream index. @item force_style Override default style or script info parameters of the subtitles. It accepts a -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On Sun, Jul 5, 2020 at 7:29 PM James Almer wrote: > > On 7/5/2020 1:58 PM, Jan Engelhardt wrote: > > > > On Sunday 2020-07-05 18:18, James Almer wrote: > > Won't that break the entire ABI of anything currently linked, and thus > would > require a major bump? > >>> > >>> Since 4.3 was sort of a break over 4.2.3 already > >> > >> No, it wasn't. > > > > Perhaps not on a high level. But for the ELF system, it was a break, > > because you used the tuple, specifically > LIBAVCODEC_58>, with two _different sets of contained symbols_. > > > > Was it the reason for blender crash? I do not know that, nor do I know the > > blender internals, but if it can be ruled out (at the very least, in the > > future) that version mixup between $buildhost and $userhost can be the > > cause of present or future crashes in blender or otherwise, I'll be damned > > if I > > didn't try. > > > > === > > > > The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: > > > > lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 > > 23:43:45 +0800) > > > > are available in the Git repository at: > > > > git://github.com/jengelh/ffmpeg master > > > > for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: > > > > build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) > > > > > > Jan Engelhardt (1): > > build: do proper ELF symbol versioning > > > > libavcodec/libavcodec.v | 254 +++- > > libavdevice/libavdevice.v | 28 +- > > libavfilter/libavfilter.v | 78 - > > libavformat/libavformat.v | 185 +++- > > libavresample/libavresample.v | 30 +- > > libavutil/libavutil.v | 555 +- > > libswresample/libswresample.v | 33 +- > > libswscale/libswscale.v | 44 ++- > > 8 files changed, 1163 insertions(+), 44 deletions(-) > > The list of functions is incomplete. After a quick look, you're for > example missing av_map_videotoolbox_format_* in libavutil. > > This is another issue that i don't know if you considered, and that's > the fact currently some symbols may not present depending on configure > time options, build environment, and target system. Videotoolbox is of > course not available for you unless you're targeting OSX. > > This afaik can be solved by ensuring they are always present, and > returning ENOSYS/NULL as required if the actual implementation is not > compiled in. > We're doing as much in a few places, but clearly not enough care was > taken to ensure that's always the case. I don't think platform specific functionality should get such a treatment. A build on the same platform should always present the same ABI, but nothing you can do will make videotoolbox function on Linux or Windows, so the symbols have no business being included. Headers for such functionality might even require OS-specific headers to be included (eg. D3D on Windows, VT on OSX), which if of course not going to work. In this case, the ABI is not dependent on configure, but on the underlying platform you are building on. Of course it should be corrected if they are purely configure-controlled, and instead perhaps moved to a platform model. - Hendrik ___ 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] Project orientation
On Sun, 5 Jul 2020 at 19:01, Kieran Kunhya wrote: > > For new contributors git send-email is annoying. For people wanting to > push, the .mbox format is annoying, Gmail doesn't support it any more. > And you can't get new contributors to start using CLI based email > clients or run their own mail server, that's not going to happen. > As a fresh contributor, setting up git send-email was a hassle, but not an insurmountable obstacle. Though I'd argue that git send-email is a bigger liability for regular developers. A lot of developers seem to be using Gmail, which means they need to have "less secure apps" enabled at the time they use git send-email. So they are forced to choose between security and convenience. I.e. having to temporarily enable "less secure apps" every time they submit, or accept the risk of weakened security for their account. > > A solution like Gitlab is the only way forward. It has worked well for > dav1d, it can run regression tests on all platforms for all commits: > https://code.videolan.org/videolan/dav1d > > Merges are done with one push of a button. Yes, the branch sprawl is > not great but it's better than now. > It has inline patch reviews which are nice. > FWIW, I'd add that branch-based PRs as implemented with GitHub greatly reduce the turnaround for receiving feedback and addressing the raised issues. All you have to do is force-push on your PR branch, and the PR is cleanly updated. This is in contrast with having to use git send-email, where the emails containing the stale patches will continue to litter patchwork and your mailboxes. Also, IIRC, when you update a branch, the regression tests for all pending PRs on the branch are automatically rerun. This should help address the conflicts in the pending PRs sooner rather than later. Not sure what the status of GitLab is wrt to these features, but I'd expect it to not be very far behind. Best regards, Manolis ___ 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] ABI break in 4.3
On 7/5/2020 1:58 PM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 18:18, James Almer wrote: Won't that break the entire ABI of anything currently linked, and thus would require a major bump? >>> >>> Since 4.3 was sort of a break over 4.2.3 already >> >> No, it wasn't. > > Perhaps not on a high level. But for the ELF system, it was a break, > because you used the tuple, specifically LIBAVCODEC_58>, with two _different sets of contained symbols_. > > Was it the reason for blender crash? I do not know that, nor do I know the > blender internals, but if it can be ruled out (at the very least, in the > future) that version mixup between $buildhost and $userhost can be the > cause of present or future crashes in blender or otherwise, I'll be damned if > I > didn't try. > > === > > The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: > > lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 > 23:43:45 +0800) > > are available in the Git repository at: > > git://github.com/jengelh/ffmpeg master > > for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: > > build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) > > > Jan Engelhardt (1): > build: do proper ELF symbol versioning > > libavcodec/libavcodec.v | 254 +++- > libavdevice/libavdevice.v | 28 +- > libavfilter/libavfilter.v | 78 - > libavformat/libavformat.v | 185 +++- > libavresample/libavresample.v | 30 +- > libavutil/libavutil.v | 555 +- > libswresample/libswresample.v | 33 +- > libswscale/libswscale.v | 44 ++- > 8 files changed, 1163 insertions(+), 44 deletions(-) The list of functions is incomplete. After a quick look, you're for example missing av_map_videotoolbox_format_* in libavutil. This is another issue that i don't know if you considered, and that's the fact currently some symbols may not present depending on configure time options, build environment, and target system. Videotoolbox is of course not available for you unless you're targeting OSX. This afaik can be solved by ensuring they are always present, and returning ENOSYS/NULL as required if the actual implementation is not compiled in. We're doing as much in a few places, but clearly not enough care was taken to ensure that's always the case. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Fair point. I've submitted a v2 where the docs reorganization is applied first. On Sun, 5 Jul 2020 at 17:30, Gyan Doshi wrote: > > > On 05-07-2020 06:35 pm, Manolis Stamatogiannakis wrote: > > Some options are common between subtitles/ass filters. > > Rather than mentioning for each option whether it is common or not, > > the options are now displayed in two separate tables. > > > > Signed-off-by: Manolis Stamatogiannakis > > --- > > doc/filters.texi | 18 +++--- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > index c962ac55b0..c4ca39cb6d 100644 > > --- a/doc/filters.texi > > +++ b/doc/filters.texi > > @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} > that corresponds to option of > > @item planes > > @end table > > > > +@anchor{ass} > > @section ass > > > > Same as the @ref{subtitles} filter, except that it doesn't require > libavcodec > > @@ -17929,15 +17930,12 @@ To enable compilation of this filter you need > to configure FFmpeg with > > libavformat to convert the passed subtitles file to ASS (Advanced > Substation > > Alpha) subtitles format. > > > > -The filter accepts the following options: > > +Common @ref{subtitles}/@ref{ass} filter options: > > > > @table @option > > @item filename, f > > Set the filename of the subtitle file to read. It must be specified. > > > > -@item shift > > -Shift subtitles timings by the specified amount. > > - > > @item original_size > > Specify the size of the original video, the video for which the ASS > file > > was composed. For the syntax of this option, check the > > @@ -17952,12 +17950,18 @@ These fonts will be used in addition to > whatever the font provider uses. > > @item alpha > > Process alpha channel, by default alpha channel is untouched. > > > > +@item shift > > +Shift subtitles timings by the specified amount. > > +@end table > > + > > +Additional options for @ref{subtitles} filter: > > + > > +@table @option > > @item charenc > > -Set subtitles input character encoding. @code{subtitles} filter only. > Only > > -useful if not UTF-8. > > +Set subtitles input character encoding. Only useful if not UTF-8. > > > > @item stream_index, si > > -Set subtitles stream index. @code{subtitles} filter only. > > +Set subtitles stream index. > > Break this off into a standalone without the shift option entry. > Then merge the doc shift entry with the code patches. > > Regards, > Gyan > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On Sunday 2020-07-05 18:18, James Almer wrote: >>> >>> Won't that break the entire ABI of anything currently linked, and thus would >>> require a major bump? >> >> Since 4.3 was sort of a break over 4.2.3 already > >No, it wasn't. Perhaps not on a high level. But for the ELF system, it was a break, because you used the tuple, specifically , with two _different sets of contained symbols_. Was it the reason for blender crash? I do not know that, nor do I know the blender internals, but if it can be ruled out (at the very least, in the future) that version mixup between $buildhost and $userhost can be the cause of present or future crashes in blender or otherwise, I'll be damned if I didn't try. === The following changes since commit c2d000ec27af1a5cd5341a67e941e0313879ab18: lavc/qsvenc_hevc: add qmax/qmin support for HEVC encoding (2020-07-05 23:43:45 +0800) are available in the Git repository at: git://github.com/jengelh/ffmpeg master for you to fetch changes up to 3ec24e4e548ecd6d4cc2f11a7d6717548abdadab: build: do proper ELF symbol versioning (2020-07-05 18:50:58 +0200) Jan Engelhardt (1): build: do proper ELF symbol versioning libavcodec/libavcodec.v | 254 +++- libavdevice/libavdevice.v | 28 +- libavfilter/libavfilter.v | 78 - libavformat/libavformat.v | 185 +++- libavresample/libavresample.v | 30 +- libavutil/libavutil.v | 555 +- libswresample/libswresample.v | 33 +- libswscale/libswscale.v | 44 ++- 8 files changed, 1163 insertions(+), 44 deletions(-) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_subtitles: Added shift option for subtitles/ass filters.
Allows shifting of subtitle display times to align them with the video. This avoids having to rewrite the subtitle file in order to display subtitles correctly when input is seeked (-ss). Also handy for minor subtitle timing corrections without rewriting the subtitles file. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 11 libavfilter/vf_subtitles.c | 54 +- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index d2b8feb14b..a8af563551 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -17936,6 +17936,9 @@ Common @ref{subtitles}/@ref{ass} filter options: @item filename, f Set the filename of the subtitle file to read. It must be specified. +@item shift +Shift subtitles timings by the specified amount. + @item original_size Specify the size of the original video, the video for which the ASS file was composed. For the syntax of this option, check the @@ -17949,6 +17952,9 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. + +@item shift +Shift subtitles timings by the specified amount. @end table Additional options for @ref{subtitles} filter: @@ -17995,6 +18001,11 @@ To make the subtitles stream from @file{sub.srt} appear in 80% transparent blue subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF' @end example +To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use: +@example +subtitles=filename=sub.srt:shift='-20\:20' +@end example + @section super2xsai Scale the input by 2x and smooth using the Super2xSaI (Scale and diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c index 1bd42391e0..09cca6aab8 100644 --- a/libavfilter/vf_subtitles.c +++ b/libavfilter/vf_subtitles.c @@ -52,6 +52,7 @@ typedef struct AssContext { char *filename; char *fontsdir; char *charenc; +int64_t shift; char *force_style; int stream_index; int alpha; @@ -66,11 +67,12 @@ typedef struct AssContext { #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM #define COMMON_OPTIONS \ -{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ -{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ +{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"shift", "shift subtitles timing", OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN, INT64_MAX, FLAGS }, \ /* libass supports a log level ranging from 0 to 7 */ static const int ass_libavfilter_log_level_map[] = { @@ -103,6 +105,11 @@ static av_cold int init(AVFilterContext *ctx) return AVERROR(EINVAL); } +if (ass->shift != 0) { +ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1, 1000)); +av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n", ass->shift/1000.0); +} + ass->library = ass_library_init(); if (!ass->library) { av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n"); @@ -227,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass); static av_cold int init_ass(AVFilterContext *ctx) { +int eid, nskip; +ASS_Event *event; AssContext *ass = ctx->priv; int ret = init(ctx); @@ -243,6 +252,24 @@ static av_cold int init_ass(AVFilterContext *ctx)
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters. Rather than mentioning for each option whether it is common or not, the options are now displayed in two separate tables. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index ad2448acb2..d2b8feb14b 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that corresponds to option of @item planes @end table +@anchor{ass} @section ass Same as the @ref{subtitles} filter, except that it doesn't require libavcodec @@ -17929,7 +17930,7 @@ To enable compilation of this filter you need to configure FFmpeg with libavformat to convert the passed subtitles file to ASS (Advanced Substation Alpha) subtitles format. -The filter accepts the following options: +Common @ref{subtitles}/@ref{ass} filter options: @table @option @item filename, f @@ -17948,13 +17949,16 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. +@end table +Additional options for @ref{subtitles} filter: + +@table @option @item charenc -Set subtitles input character encoding. @code{subtitles} filter only. Only -useful if not UTF-8. +Set subtitles input character encoding. Only useful if not UTF-8. @item stream_index, si -Set subtitles stream index. @code{subtitles} filter only. +Set subtitles stream index. @item force_style Override default style or script info parameters of the subtitles. It accepts a -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: x264 is practically feature complete, but x262 still miss some things, like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, but it is still not packaged in e.g. Ubuntu, so users are stuck with the - in some ways - inferior mpeg2 encoder of ffmpeg. The point I am trying to make is that you and some other people made a fast and modern mpeg2 encoder, in some ways superior to existing open source alternatives, but very few people is using it because it was not merged to a bigger/more popular project like x264 or ffmpeg. So it receives no mainteance, no distribution support, no user base and ultimately no further development. Or at least that is how I see it. People aren't using it because people don't use MPEG-2. There is no maintenance to be done on a format that's 25 years old, do you want me to randomly change cosmetics to make you feel happy? If you'd get off your high horse for once, that would make me feel happy. I know of people using it 24/7 for many years and have had no issues. Have you opened bug reports on x264-devel about the issues you see? MPEG-2 4:2:2 interlaced users are infinitesimally small and professional users. If you want your pet feature to work, lo and behold you have to work on it or pay someone to work on it. If you want people to work on it out of nowhere then you are part of the Free Rider Problem: https://en.wikipedia.org/wiki/Free-rider_problem I don't think I told anybody to implement the missing features for free. But I do believe that payed or free development has a higher chance of happening if existing code is already available in a popular package/project. And yes, I believe that some "maintenance burden" should be accepted by the base project to give more chance to further advancement, payed or free. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On 7/5/2020 12:46 PM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote: >>> LIBAVCODEC_58 { >>>global: >>>av_foo; >>>av_bar; >>>av_whatever_else_there_is;... >>>local: >>>*; >>> }; >>> LIBAVCODEC_58.91 { >>>global: >>>avpriv_mpeg4audio_get_config2; >>> } LIBAVCODEC_58; >>> LIBAVCODEC_58.123 { /* future example */ >>>global: >>>avblahblah; >>> } LIBAVCODEC_58.91; >> >> Won't that break the entire ABI of anything currently linked, and thus would >> require a major bump? > > Since 4.3 was sort of a break over 4.2.3 already No, it wasn't. No field offsets were changed, no public structs where their size is part of the ABI were altered, and no public symbols were removed. The situation is exactly the same as when 4.2 was released after 4.1, and every other release before those. If you can run 4.3 at runtime on a program that linked to 4.2, then it's working as intended. Attempting the inverse is not and will never be allowed or considered a valid scenario. And i want to stress the fact that the reported Chromium breakage and most assuredly also the Blender breakage are unrelated to what you're arguing about in this thread. > , the situation is that: > > * $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case >avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or > > * $nextrelease can be compatible with 4.3's idea of the ABI, in which case >avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58. > > Each of the two prior options is equally non-compelling. "58" is tarnished > already. As "tarnished" as every previous soname was, if we go by your definition of breakage. I don't recall it ever being an issue for anyone until now. This nonetheless is a good proposal and can be considered for the next soname bump, if it can help prevent people using the wrong release at runtime. > What software generally does at this point — ffmpeg is not the first > project to have these issues — is to bump and start fresh. > > >> Generally, this seems incredibly hard to maintain and will very likely cause >> confusion every time someone adds stuff in the future. > > How often do exported functions get added? Between 4.2.3 and 4.3, > that was _just one_, and that one was even an avpriv_* (which > probably shouldn't have been exported given its "priv" nature?!). avpriv_* are inter library communication functions. An unfortunate consequence of having the project split across half a dozen libraries. They need to be exported, but not exposed in the installed headers. ___ 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] ABI break in 4.3
On 05.07.2020 17:46, Jan Engelhardt wrote: On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote: LIBAVCODEC_58 { global: av_foo; av_bar; av_whatever_else_there_is;... local: *; }; LIBAVCODEC_58.91 { global: avpriv_mpeg4audio_get_config2; } LIBAVCODEC_58; LIBAVCODEC_58.123 { /* future example */ global: avblahblah; } LIBAVCODEC_58.91; Won't that break the entire ABI of anything currently linked, and thus would require a major bump? Since 4.3 was sort of a break over 4.2.3 already, the situation is that: It wasn't. Structs expanding at the end have occurred multiple times in the past already, and are documented as not having their size be part of the ABI/API. Also, no functions were removed. * $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or * $nextrelease can be compatible with 4.3's idea of the ABI, in which case avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58. Each of the two prior options is equally non-compelling. "58" is tarnished already. What software generally does at this point — ffmpeg is not the first project to have these issues — is to bump and start fresh. Generally, this seems incredibly hard to maintain and will very likely cause confusion every time someone adds stuff in the future. How often do exported functions get added? Between 4.2.3 and 4.3, that was _just one_, and that one was even an avpriv_* (which probably shouldn't have been exported given its "priv" nature?!). Sometimes the libraries have functions they access from each other that are not meant to be public, but still need to be exported. ___ 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 submission questions
Thanks for the responses Hongcheng and Steinar! I'll use '-n' with git format-patch if I need to resubmit. Patches already work as Steinar describes. In the meantime patch 2/3 found its way to patchwork. The delay was probably due to some email processing hiccup. So all is good! Cheers, Manolis On Sun, 5 Jul 2020 at 16:06, Steinar H. Gunderson < steinar+ffm...@gunderson.no> wrote: > On Sun, Jul 05, 2020 at 03:42:34PM +0200, Manolis Stamatogiannakis wrote: > > Q2: In a patchset consisting of several commits, is each commit expected > to > > be "standalone"? I.e. does it have to apply cleanly without depending on > > the previous commits in the patchset? > > No, but it has to compile and work even if not applying any of the latter > patches. > (This increases reviewability and bisectability.) AFAIK Patchwork doesn't > enforce this, though. > > /* Steinar */ > -- > Homepage: https://www.sesse.net/ > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/4] kmsgrab: Use GetFB2 if available
The most useful feature here is the ability to automatically extract the framebuffer format and modifiers. It also makes support for multi-plane framebuffers possible, though they will need to be added to the format table to work (not tested by me). This requires libdrm 2.4.101 (from April 2020) to build, so it includes a configure check to allow compatibility with existing distributions. Even with libdrm support, it still won't do anything at runtime if you are running Linux < 5.7 (before June 2020). --- This has been hanging around for a while waiting for the GETFB2 ioctl() to actually make it into stable Linux, which it now is with 5.7. configure | 4 + libavdevice/kmsgrab.c | 221 +- 2 files changed, 203 insertions(+), 22 deletions(-) diff --git a/configure b/configure index bdfd731602..2900acc687 100755 --- a/configure +++ b/configure @@ -2323,6 +2323,7 @@ HAVE_LIST=" $THREADS_LIST $TOOLCHAIN_FEATURES $TYPES_LIST +libdrm_getfb2 makeinfo makeinfo_html opencl_d3d11 @@ -6631,6 +6632,9 @@ test_cpp <= 0.35.0" "va/va.h" vaInitialize diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c index 47ba15ca07..3e89c3f445 100644 --- a/libavdevice/kmsgrab.c +++ b/libavdevice/kmsgrab.c @@ -27,6 +27,11 @@ #include #include +// Required for compatibility when building against libdrm < 2.4.83. +#ifndef DRM_FORMAT_MOD_INVALID +#define DRM_FORMAT_MOD_INVALID ((1ULL << 56) - 1) +#endif + #include "libavutil/hwcontext.h" #include "libavutil/hwcontext_drm.h" #include "libavutil/internal.h" @@ -45,6 +50,7 @@ typedef struct KMSGrabContext { AVBufferRef *device_ref; AVHWDeviceContext *device; AVDRMDeviceContext *hwctx; +int fb2_available; AVBufferRef *frames_ref; AVHWFramesContext *frames; @@ -68,8 +74,10 @@ typedef struct KMSGrabContext { static void kmsgrab_free_desc(void *opaque, uint8_t *data) { AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor*)data; +int i; -close(desc->objects[0].fd); +for (i = 0; i < desc->nb_objects; i++) +close(desc->objects[i].fd); av_free(desc); } @@ -142,6 +150,114 @@ fail: return err; } +#if HAVE_LIBDRM_GETFB2 +static int kmsgrab_get_fb2(AVFormatContext *avctx, + drmModePlane *plane, + AVDRMFrameDescriptor *desc) +{ +KMSGrabContext *ctx = avctx->priv_data; +drmModeFB2 *fb; +int err, i, nb_objects; + +fb = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id); +if (!fb) { +av_log(avctx, AV_LOG_ERROR, "Failed to get framebuffer " + "%"PRIu32".\n", plane->fb_id); +return AVERROR(EIO); +} +if (fb->pixel_format != ctx->drm_format) { +av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer " + "format changed: now %"PRIx32".\n", + ctx->plane_id, fb->pixel_format); +err = AVERROR(EIO); +goto fail; +} +if (fb->modifier != ctx->drm_format_modifier) { +av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer " + "format modifier changed: now %"PRIx64".\n", + ctx->plane_id, fb->modifier); +err = AVERROR(EIO); +goto fail; +} +if (fb->width != ctx->width || fb->height != ctx->height) { +av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer " + "dimensions changed: now %"PRIu32"x%"PRIu32".\n", + ctx->plane_id, fb->width, fb->height); +err = AVERROR(EIO); +goto fail; +} +if (!fb->handles[0]) { +av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer.\n"); +err = AVERROR(EIO); +goto fail; +} + +*desc = (AVDRMFrameDescriptor) { +.nb_layers = 1, +.layers[0] = { +.format = ctx->drm_format, +}, +}; + +nb_objects = 0; +for (i = 0; i < 4 && fb->handles[i]; i++) { +size_t size; +int dup = 0, j, obj; + +size = fb->offsets[i] + fb->height * fb->pitches[i]; + +for (j = 0; j < i; j++) { +if (fb->handles[i] == fb->handles[j]) { +dup = 1; +break; +} +} +if (dup) { +obj = desc->layers[0].planes[j].object_index; + +if (desc->objects[j].size < size) +desc->objects[j].size = size; + +desc->layers[0].planes[i] = (AVDRMPlaneDescriptor) { +.object_index = obj, +.offset = fb->offsets[i], +.pitch= fb->pitches[i], +}; + +} else { +int fd; +err = drmPrimeHandleToFD(ctx->hwctx->fd, fb->handles[i], + O_RDONLY, &fd); +if (err < 0) { +err = AVERROR(errno); +av_log(avctx, AV_LOG_ERROR, "Failed to get PRIME fd from " + "framebuffer handle: %
[FFmpeg-devel] [PATCH 3/4] kmsgrab: Don't require the user to set framebuffer format
This is provided by GetFB2, but we still need the option for cases where that isn't available. --- libavdevice/kmsgrab.c | 55 +++ 1 file changed, 40 insertions(+), 15 deletions(-) diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c index 3e89c3f445..b859f202aa 100644 --- a/libavdevice/kmsgrab.c +++ b/libavdevice/kmsgrab.c @@ -405,18 +405,6 @@ static av_cold int kmsgrab_read_header(AVFormatContext *avctx) AVStream *stream; int err, i; -for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) { -if (kmsgrab_formats[i].pixfmt == ctx->format) { -ctx->drm_format = kmsgrab_formats[i].drm_format; -break; -} -} -if (i >= FF_ARRAY_ELEMS(kmsgrab_formats)) { -av_log(avctx, AV_LOG_ERROR, "Unsupported format %s.\n", - av_get_pix_fmt_name(ctx->format)); -return AVERROR(EINVAL); -} - err = av_hwdevice_ctx_create(&ctx->device_ref, AV_HWDEVICE_TYPE_DRM, ctx->device_path, NULL, 0); if (err < 0) { @@ -530,9 +518,25 @@ static av_cold int kmsgrab_read_header(AVFormatContext *avctx) err = AVERROR(EINVAL); goto fail; } -if (ctx->drm_format != fb2->pixel_format) { + +for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) { +if (kmsgrab_formats[i].drm_format == fb2->pixel_format) { +if (ctx->format != AV_PIX_FMT_NONE && +ctx->format != kmsgrab_formats[i].pixfmt) { +av_log(avctx, AV_LOG_ERROR, "Framebuffer pixel format " + "%"PRIx32" does not match expected format.\n", + fb2->pixel_format); +err = AVERROR(EINVAL); +goto fail; +} +ctx->drm_format = fb2->pixel_format; +ctx->format = kmsgrab_formats[i].pixfmt; +break; +} +} +if (i == FF_ARRAY_ELEMS(kmsgrab_formats)) { av_log(avctx, AV_LOG_ERROR, "Framebuffer pixel format " - "%"PRIx32" does not match expected format.\n", + "%"PRIx32" is not a known supported format.\n", fb2->pixel_format); err = AVERROR(EINVAL); goto fail; @@ -547,11 +551,32 @@ static av_cold int kmsgrab_read_header(AVFormatContext *avctx) } else { ctx->drm_format_modifier = fb2->modifier; } +av_log(avctx, AV_LOG_VERBOSE, "Format is %s, from " + "DRM format %"PRIx32" modifier %"PRIx64".\n", + av_get_pix_fmt_name(ctx->format), + ctx->drm_format, ctx->drm_format_modifier); + ctx->fb2_available = 1; } #endif if (!ctx->fb2_available) { +if (ctx->format == AV_PIX_FMT_NONE) { +// Backward compatibility: assume BGR0 if no format supplied. +ctx->format = AV_PIX_FMT_BGR0; +} +for (i = 0; i < FF_ARRAY_ELEMS(kmsgrab_formats); i++) { +if (kmsgrab_formats[i].pixfmt == ctx->format) { +ctx->drm_format = kmsgrab_formats[i].drm_format; +break; +} +} +if (i >= FF_ARRAY_ELEMS(kmsgrab_formats)) { +av_log(avctx, AV_LOG_ERROR, "Unsupported format %s.\n", + av_get_pix_fmt_name(ctx->format)); +return AVERROR(EINVAL); +} + fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id); if (!fb) { err = errno; @@ -642,7 +667,7 @@ static const AVOption options[] = { { .str = "/dev/dri/card0" }, 0, 0, FLAGS }, { "format", "Pixel format for framebuffer", OFFSET(format), AV_OPT_TYPE_PIXEL_FMT, - { .i64 = AV_PIX_FMT_BGR0 }, 0, UINT32_MAX, FLAGS }, + { .i64 = AV_PIX_FMT_NONE }, -1, INT32_MAX, FLAGS }, { "format_modifier", "DRM format modifier for framebuffer", OFFSET(drm_format_modifier), AV_OPT_TYPE_INT64, { .i64 = DRM_FORMAT_MOD_INVALID }, 0, INT64_MAX, FLAGS }, -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavfi/vf_vpp_qsv: fix the infinite loop while framerate lower than input
> > I can't see the benefit to use MSDK framerate conversion function. Is > > it a good idea to drop it and use ffmpeg native fps filter instead? > > Reconsidering this, leaving the filter buggy doesn't seem to be comfortable > to me, > hence IMHO it'll be better to have this bug fixed. My suggestion would be just delete MSDK framerate conversion instead of patch it. Will send a patch if nobody against. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/4] kmsgrab: Refactor and clean error cases
--- libavdevice/kmsgrab.c | 151 ++ 1 file changed, 93 insertions(+), 58 deletions(-) diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c index d0de774871..47ba15ca07 100644 --- a/libavdevice/kmsgrab.c +++ b/libavdevice/kmsgrab.c @@ -81,70 +81,42 @@ static void kmsgrab_free_frame(void *opaque, uint8_t *data) av_frame_free(&frame); } -static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt) +static int kmsgrab_get_fb(AVFormatContext *avctx, + drmModePlane *plane, + AVDRMFrameDescriptor *desc) { KMSGrabContext *ctx = avctx->priv_data; -drmModePlane *plane; -drmModeFB *fb; -AVDRMFrameDescriptor *desc; -AVFrame *frame; -int64_t now; +drmModeFB *fb = NULL; int err, fd; -now = av_gettime(); -if (ctx->frame_last) { -int64_t delay; -while (1) { -delay = ctx->frame_last + ctx->frame_delay - now; -if (delay <= 0) -break; -av_usleep(delay); -now = av_gettime(); -} -} -ctx->frame_last = now; - -plane = drmModeGetPlane(ctx->hwctx->fd, ctx->plane_id); -if (!plane) { -av_log(avctx, AV_LOG_ERROR, "Failed to get plane " - "%"PRIu32".\n", ctx->plane_id); -return AVERROR(EIO); -} -if (!plane->fb_id) { -av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" no longer has " - "an associated framebuffer.\n", ctx->plane_id); -return AVERROR(EIO); -} - fb = drmModeGetFB(ctx->hwctx->fd, plane->fb_id); if (!fb) { av_log(avctx, AV_LOG_ERROR, "Failed to get framebuffer " "%"PRIu32".\n", plane->fb_id); -return AVERROR(EIO); +err = AVERROR(EIO); +goto fail; } if (fb->width != ctx->width || fb->height != ctx->height) { av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" framebuffer " "dimensions changed: now %"PRIu32"x%"PRIu32".\n", ctx->plane_id, fb->width, fb->height); -return AVERROR(EIO); +err = AVERROR(EIO); +goto fail; } if (!fb->handle) { av_log(avctx, AV_LOG_ERROR, "No handle set on framebuffer.\n"); -return AVERROR(EIO); +err = AVERROR(EIO); +goto fail; } err = drmPrimeHandleToFD(ctx->hwctx->fd, fb->handle, O_RDONLY, &fd); if (err < 0) { -err = errno; +err = AVERROR(errno); av_log(avctx, AV_LOG_ERROR, "Failed to get PRIME fd from " "framebuffer handle: %s.\n", strerror(errno)); -return AVERROR(err); +goto fail; } -desc = av_mallocz(sizeof(*desc)); -if (!desc) -return AVERROR(ENOMEM); - *desc = (AVDRMFrameDescriptor) { .nb_objects = 1, .objects[0] = { @@ -164,31 +136,92 @@ static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt) }, }; +err = 0; +fail: +drmModeFreeFB(fb); +return err; +} + +static int kmsgrab_read_packet(AVFormatContext *avctx, AVPacket *pkt) +{ +KMSGrabContext *ctx = avctx->priv_data; +drmModePlane *plane = NULL; +AVDRMFrameDescriptor *desc = NULL; +AVFrame *frame = NULL; +int64_t now; +int err; + +now = av_gettime(); +if (ctx->frame_last) { +int64_t delay; +while (1) { +delay = ctx->frame_last + ctx->frame_delay - now; +if (delay <= 0) +break; +av_usleep(delay); +now = av_gettime(); +} +} +ctx->frame_last = now; + +plane = drmModeGetPlane(ctx->hwctx->fd, ctx->plane_id); +if (!plane) { +av_log(avctx, AV_LOG_ERROR, "Failed to get plane " + "%"PRIu32".\n", ctx->plane_id); +err = AVERROR(EIO); +goto fail; +} +if (!plane->fb_id) { +av_log(avctx, AV_LOG_ERROR, "Plane %"PRIu32" no longer has " + "an associated framebuffer.\n", ctx->plane_id); +err = AVERROR(EIO); +goto fail; +} + +desc = av_mallocz(sizeof(*desc)); +if (!desc) { +err = AVERROR(ENOMEM); +goto fail; +} + +err = kmsgrab_get_fb(avctx, plane, desc); +if (err < 0) +goto fail; + frame = av_frame_alloc(); -if (!frame) -return AVERROR(ENOMEM); +if (!frame) { +err = AVERROR(ENOMEM); +goto fail; +} frame->hw_frames_ctx = av_buffer_ref(ctx->frames_ref); -if (!frame->hw_frames_ctx) -return AVERROR(ENOMEM); +if (!frame->hw_frames_ctx) { +err = AVERROR(ENOMEM); +goto fail; +} frame->buf[0] = av_buffer_create((uint8_t*)desc, sizeof(*desc), &kmsgrab_free_desc, avctx, 0); -if (!frame->buf[0]) -return AVERROR(ENOMEM); +if (!frame->buf[0]) { +err = AVERROR(ENOMEM); +goto fail; +} frame->dat
[FFmpeg-devel] [PATCH 4/4] doc/indevs: Note improved behaviour of kmsgrab with Linux 5.7
--- doc/indevs.texi | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/indevs.texi b/doc/indevs.texi index 0f33fc66d8..4d2312e201 100644 --- a/doc/indevs.texi +++ b/doc/indevs.texi @@ -889,11 +889,15 @@ If you don't understand what all of that means, you probably don't want this. L DRM device to capture on. Defaults to @option{/dev/dri/card0}. @item format -Pixel format of the framebuffer. Defaults to @option{bgr0}. +Pixel format of the framebuffer. This can be autodetected if you are running Linux 5.7 +or later, but needs to be provided for earlier versions. Defaults to @option{bgr0}, +which is the most common format used by the Linux console and Xorg X server. @item format_modifier Format modifier to signal on output frames. This is necessary to import correctly into -some APIs, but can't be autodetected. See the libdrm documentation for possible values. +some APIs. It can be autodetected if you are running Linux 5.7 or later, but will need +to be provided explicitly when needed in earlier versions. See the libdrm documentation +for possible values. @item crtc_id KMS CRTC ID to define the capture source. The first active plane on the given CRTC -- 2.27.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On Sunday 2020-07-05 16:43, Timo Rothenpieler wrote: >> LIBAVCODEC_58 { >>global: >>av_foo; >>av_bar; >>av_whatever_else_there_is;... >>local: >>*; >> }; >> LIBAVCODEC_58.91 { >>global: >>avpriv_mpeg4audio_get_config2; >> } LIBAVCODEC_58; >> LIBAVCODEC_58.123 { /* future example */ >>global: >>avblahblah; >> } LIBAVCODEC_58.91; > > Won't that break the entire ABI of anything currently linked, and thus would > require a major bump? Since 4.3 was sort of a break over 4.2.3 already, the situation is that: * $nextrelease can be compatible with 4.2.3's idea of the ABI, in which case avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58.91, or * $nextrelease can be compatible with 4.3's idea of the ABI, in which case avpriv_mpeg4audio_get_config2 should be placed inside LIBAVCODEC_58. Each of the two prior options is equally non-compelling. "58" is tarnished already. What software generally does at this point — ffmpeg is not the first project to have these issues — is to bump and start fresh. > Generally, this seems incredibly hard to maintain and will very likely cause > confusion every time someone adds stuff in the future. How often do exported functions get added? Between 4.2.3 and 4.3, that was _just one_, and that one was even an avpriv_* (which probably shouldn't have been exported given its "priv" nature?!). ___ 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] ABI break in 4.3
Am So., 5. Juli 2020 um 16:44 Uhr schrieb Timo Rothenpieler : > Generally, this seems incredibly hard to maintain and will very likely > cause confusion every time someone adds stuff in the future. True, and this is while this will not even reach the committee. Carl Eugen ___ 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] ABI break in 4.3
Am So., 5. Juli 2020 um 16:46 Uhr schrieb Timo Rothenpieler : > > On 05.07.2020 16:18, Carl Eugen Hoyos wrote: > > Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt > > : > > > >> This crash is due to Chromium using av_max_alloc in an undocumented way; > >> more exactly, the Chromium FFmpeg fork changes the allocation functions > >> so that if max_alloc_size is zero, the limit is completely disabled; and > >> of course it also sets the limit to zero. Up until 4.2, this worked with > >> a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead > >> max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 > >> this is no longer so. > > > > I think it is not immediately obvious that this is a (severe!) issue in > > Chromium which basically disabled a security feature of FFmpeg > > that was intentionally set to a very conservative (read: not soo > > secure) value but was completely disabled by somebody who > > misunderstood the feature (and failed to ask, I mention this > > because this person's understanding would have implied that we > > have no clue in C programming whatsoever). > > > > At least one of the "downstream" fixes I saw in the last weeks simply > > repeat this failure by again removing the security feature instead of > > removing the wrong call from Chromium. > > > > I am not sure if it really is our responsibility to explain to downstream > > that valid multimedia files theoretically can allocate arbitrary amounts > > of memory but that a responsible caller has to limit this amount for > > nearly every theoretical use case, the more so for browser decoding. > > > > Carl Eugen > > Chrome is using a custom allocator, that crashes the entire application > on OOM rather than returning NULL. > So it's not a security issue in their case. I understood the situation differently (because this is not about oom): FFmpeg (by default) limits the possible allocation of a single call to malloc() - independently of the used memory allocator. The default limit is extremely high and it is difficult to image a useful file that reaches the limit. Somebody at Google thought that the "limitation" means that libavutil returns only a block of "limit" if a block bigger than limit was requested and therefore decided to remove the limit. Apart from the fact that this would be a very severe issue in FFmpeg that this Google employee decided not to report to us, in addition the removal of the limit means that it is possible to allocate nearly the complete memory from a media file within the browser which clearly is a security issue in my opinion. (I have no idea if their allocator limits the max possible allocation anyway but even if it does, there should be no reason for their change.) Carl Eugen ___ 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] avfilter/vf_subtitles: Reorganized subtitles filter options.
On 05-07-2020 06:35 pm, Manolis Stamatogiannakis wrote: Some options are common between subtitles/ass filters. Rather than mentioning for each option whether it is common or not, the options are now displayed in two separate tables. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index c962ac55b0..c4ca39cb6d 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that corresponds to option of @item planes @end table +@anchor{ass} @section ass Same as the @ref{subtitles} filter, except that it doesn't require libavcodec @@ -17929,15 +17930,12 @@ To enable compilation of this filter you need to configure FFmpeg with libavformat to convert the passed subtitles file to ASS (Advanced Substation Alpha) subtitles format. -The filter accepts the following options: +Common @ref{subtitles}/@ref{ass} filter options: @table @option @item filename, f Set the filename of the subtitle file to read. It must be specified. -@item shift -Shift subtitles timings by the specified amount. - @item original_size Specify the size of the original video, the video for which the ASS file was composed. For the syntax of this option, check the @@ -17952,12 +17950,18 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. +@item shift +Shift subtitles timings by the specified amount. +@end table + +Additional options for @ref{subtitles} filter: + +@table @option @item charenc -Set subtitles input character encoding. @code{subtitles} filter only. Only -useful if not UTF-8. +Set subtitles input character encoding. Only useful if not UTF-8. @item stream_index, si -Set subtitles stream index. @code{subtitles} filter only. +Set subtitles stream index. Break this off into a standalone without the shift option entry. Then merge the doc shift entry with the code patches. Regards, Gyan ___ 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] Project orientation
sön 2020-07-05 klockan 14:28 +0200 skrev Marton Balint: > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: > > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > > > > > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > > > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > > > > wrote: > > > > > > [...] > > > > > > > At some point, the project needs to decide what is in and what is > > > > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > > > > externally. It's not a problem to say "No" to a feature, > > > > > > > especially when, the number of people using that feature is under > > > > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > > > > anyway. > > > > > > > > > > > > what we need is not to say "no" but to say > > > > > > "use this API, implement the module in your own repository, and > > > > > > register your module there" > > > > > > > > > > Once again, Michael, we agree, on this topic. > > > > > > > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > > > > useful". > > > > > > > > I'd like to express a general agreement with this point. The project > > > > has experienced quite a lot of scope creep lately. We don't have to > > > > accommodate everyone, especially with the finite number of developers > > > > available. > > > > > > > > We can encourage people to maintain their own forks of FFmpeg, see for > > > > example FFmbc. > > > > > > And see how dead that project is. Or ffserver. Or x262. > > > > That may be because Baptiste is focusing on other things. The code is > > still there. It still compiles. > > That is the point. Baptise is focusing on other things, and the code he > used to do rots in an external repository. I guess most of the features he > implemented crept back to ffmpeg (I did the backporting of some features > myself), but still, it was duplicate work. It's not duplicate work if the work wouldn't have happened at all in the first place. We could probably work toward merging more features from FFmbc into FFmpeg since presumably Baptiste doesn't depend on the license trolling aspect of the former any more. I could probably do it if Baptiste would be willing to dual license them and we could find a bit of funding for it. > Forks are good, if you are submitting the code back to master. If not, > then forks are not so good because they often dont't live on alone and you > lose the features in it. Here is where scope/feature creep becomes a problem. Features aren't free. Every feature is a liability and needs to be maintained. I for one don't have infinite time to spend. This is why I focus mostly on MXF. > > Would you want something experimental like x262 to be part of the > > FFmpeg codebase, for us to have to maintain forever? If you don't limit > > scope then that is what would happen. > > x262 is another example of a fork, where the fork alone was not > popular/interesting enough to live on. If it were merged to x264, I am > fairly certain it would not be experimental anymore, and we'd have an > MPEG2 encoder which would scale much better to multiple cores than what we > have now in ffmpeg. > > So having something merged and maintained in ffmpeg has the benefit that > more people have the ability to test the code and to develop it. Also it > is more likely for the project to get developers if their code live in the > core ffmpeg respository. I also don't see that maitenance burden to be so > huge. And usefulness is also questionable. I think it is safe to say that > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... (and I quite liked Lego Racers!!!). > > So my opinion would be to be inclusive when merging stuff. I am not saying > everything has to be merged, I rejected not very long ago the NIT table > parsing or the EPG "data decoder", these were really out of scope and more > importantly out of the current capabilites of the framework. And if > distros are worried about dependencies, they can always make two packages, > a normal and a full. Decent enough points. I just prefer being a bit more conservative with what gets in, due to the maintainance burden. > And I'd also like to point to the linux kernel as an example of a > monolitic code repository which seems to work quite well. Kernel development being quite well-delegated may contribute to this. Perhaps FFmpeg should move more in such a direction? Might be interesting. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, 5 Jul 2020, Kieran Kunhya wrote: Would you want something experimental like x262 to be part of the > FFmpeg codebase, for us to have to maintain forever? If you don't limit > scope then that is what would happen. x262 is another example of a fork, where the fork alone was not popular/interesting enough to live on. If it were merged to x264, I am fairly certain it would not be experimental anymore, and we'd have an MPEG2 encoder which would scale much better to multiple cores than what we have now in ffmpeg. Highly unlikely, x264 development has essentially ground to a halt. And people use H.264 a lot still. x262 works well enough for an old format like MPEG-2. There's no real need to develop it unless someone needs to eke out an extra 2% compression because they have a million MPEG-2 receivers they can't change. x264 is practically feature complete, but x262 still miss some things, like 4:2:2 interlaced. Sure, x262 can work well enough for some use cases, but it is still not packaged in e.g. Ubuntu, so users are stuck with the - in some ways - inferior mpeg2 encoder of ffmpeg. The point I am trying to make is that you and some other people made a fast and modern mpeg2 encoder, in some ways superior to existing open source alternatives, but very few people is using it because it was not merged to a bigger/more popular project like x264 or ffmpeg. So it receives no mainteance, no distribution support, no user base and ultimately no further development. Or at least that is how I see it. Regards, Marton The original x264 developers didn't want a merge back by the way. And I'd also like to point to the linux kernel as an example of a monolitic code repository which seems to work quite well. Exception that proves the rule. A lot of developers there are paid full time to work on the kernel and just do cleanup, merge old patches etc. Kieran ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
sön 2020-07-05 klockan 14:10 +0200 skrev Jan Engelhardt: > On Sunday 2020-07-05 13:39, Tomas Härdin wrote: > > > > Downgrading to a .so file with a lower minor version number than > > > > the program is built against can never be expected to work. Else > > > > we couldn't add new functions without a major bump. > > > > > > It requires the use ELF symbol versions -- which ffmpeg fails to > > > do properly.[...] > > > > This is a fair point. I didn't actually know the loader can do stuff > > like this, sounds super handy. How hard would it be to get that going? > > Change libavcodec.v to > > LIBAVCODEC_58 { > global: > av_foo; > av_bar; > av_whatever_else_there_is;... > local: > *; > }; > LIBAVCODEC_58.91 { > global: > avpriv_mpeg4audio_get_config2; > } LIBAVCODEC_58; > LIBAVCODEC_58.123 { /* future example */ > global: > avblahblah; > } LIBAVCODEC_58.91; > > Repeat likewise for other .v files. The file is no longer a template. The > automatic substitution of "_MAJOR" by the build system needs to cease. Version > numbers in the file are supposed to be fixed (among the set of all .so files > that share a SONAME). Interesting. This also makes what's changed between versions more explicit. Can this be checked by machine as well? Like having a post- receive hook that makes sure the .v files are consistent, or maybe have FATE check it somehow. This probably needs to be passed through the technical committee. But I don't think it'll break the ABI like Timo suggests, if we bump minor at the same time. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On Sun, 5 Jul 2020, Timo Rothenpieler wrote: On 05.07.2020 16:18, Carl Eugen Hoyos wrote: Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : This crash is due to Chromium using av_max_alloc in an undocumented way; more exactly, the Chromium FFmpeg fork changes the allocation functions so that if max_alloc_size is zero, the limit is completely disabled; and of course it also sets the limit to zero. Up until 4.2, this worked with a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen Chrome is using a custom allocator, that crashes the entire application on OOM rather than returning NULL. So it's not a security issue in their case. This is based on the assumption that a libav* function only returns NULL if there is a memory allocation error. That is simply not true. I am sure we can find a function in the codebase which returns NULL because of invalid arguments, or some other condition. IMHO they should not make this assumption. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On 05.07.2020 16:18, Carl Eugen Hoyos wrote: Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : This crash is due to Chromium using av_max_alloc in an undocumented way; more exactly, the Chromium FFmpeg fork changes the allocation functions so that if max_alloc_size is zero, the limit is completely disabled; and of course it also sets the limit to zero. Up until 4.2, this worked with a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen Chrome is using a custom allocator, that crashes the entire application on OOM rather than returning NULL. So it's not a security issue in their case. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
On 05.07.2020 14:10, Jan Engelhardt wrote: On Sunday 2020-07-05 13:39, Tomas Härdin wrote: Downgrading to a .so file with a lower minor version number than the program is built against can never be expected to work. Else we couldn't add new functions without a major bump. It requires the use ELF symbol versions -- which ffmpeg fails to do properly.[...] This is a fair point. I didn't actually know the loader can do stuff like this, sounds super handy. How hard would it be to get that going? Change libavcodec.v to LIBAVCODEC_58 { global: av_foo; av_bar; av_whatever_else_there_is;... local: *; }; LIBAVCODEC_58.91 { global: avpriv_mpeg4audio_get_config2; } LIBAVCODEC_58; LIBAVCODEC_58.123 { /* future example */ global: avblahblah; } LIBAVCODEC_58.91; Repeat likewise for other .v files. The file is no longer a template. The automatic substitution of "_MAJOR" by the build system needs to cease. Version numbers in the file are supposed to be fixed (among the set of all .so files that share a SONAME). Won't that break the entire ABI of anything currently linked, and thus would require a major bump? Generally, this seems incredibly hard to maintain and will very likely cause confusion every time someone adds stuff in the future. ___ 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] How to resolve "ERROR: sdl2 requested but not found" when cross compile?
Am So., 5. Juli 2020 um 12:46 Uhr schrieb JACKY_ZZ[猫猫爱吃鱼] : > > but I got "ERROR: sdl2 requested but not found" message Was this the only message printed on the console? Carl Eugen ___ 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] ABI break in 4.3
Am So., 5. Juli 2020 um 10:43 Uhr schrieb Jan Engelhardt : > A program may have been built with 4.3 but is combined > with 4.2.3 at runtime While I can only speak for myself, this is not only not supported, it is also something that FFmpeg will not support in the future. Carl Eugen , then this can happen: > > a = avcodec_dct_alloc(); // allocates 896 > #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED > a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 > #endif > > If the API should not be used this way, it should not offer this way. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] ABI break in 4.3
Am So., 5. Juli 2020 um 01:38 Uhr schrieb Andreas Rheinhardt : > This crash is due to Chromium using av_max_alloc in an undocumented way; > more exactly, the Chromium FFmpeg fork changes the allocation functions > so that if max_alloc_size is zero, the limit is completely disabled; and > of course it also sets the limit to zero. Up until 4.2, this worked with > a normal FFmpeg, because FFmpeg didn't max_alloc_size as is, but instead > max_alloc_size - 32. But since 731c77589841c02e014aa7f8285dcfc8b20f2ee5 > this is no longer so. I think it is not immediately obvious that this is a (severe!) issue in Chromium which basically disabled a security feature of FFmpeg that was intentionally set to a very conservative (read: not soo secure) value but was completely disabled by somebody who misunderstood the feature (and failed to ask, I mention this because this person's understanding would have implied that we have no clue in C programming whatsoever). At least one of the "downstream" fixes I saw in the last weeks simply repeat this failure by again removing the security feature instead of removing the wrong call from Chromium. I am not sure if it really is our responsibility to explain to downstream that valid multimedia files theoretically can allocate arbitrary amounts of memory but that a responsible caller has to limit this amount for nearly every theoretical use case, the more so for browser decoding. Carl Eugen ___ 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 submission questions
On Sun, Jul 05, 2020 at 03:42:34PM +0200, Manolis Stamatogiannakis wrote: > Q2: In a patchset consisting of several commits, is each commit expected to > be "standalone"? I.e. does it have to apply cleanly without depending on > the previous commits in the patchset? No, but it has to compile and work even if not applying any of the latter patches. (This increases reviewability and bisectability.) AFAIK Patchwork doesn't enforce this, though. /* Steinar */ -- Homepage: https://www.sesse.net/ ___ 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] ABI break in 4.3
On 7/5/2020 5:43 AM, Jan Engelhardt wrote: > > On Sunday 2020-07-05 01:04, James Almer wrote: >> On 7/4/2020 7:54 PM, Jan Engelhardt wrote: >>> @@ -67,6 +67,10 @@ typedef struct AVDCT { >>> + >>> +void (*get_pixels_unaligned)(int16_t *block /* align 16 */, >>> + const uint8_t *pixels, >>> + ptrdiff_t line_size); >>> } AVDCT; >> >> Neither of these are breaks as sizeof(AVCodecContext) and sizeof(AVDCT) >> are not part of the ABI. Both structs are meant to be allocated using >> avcodec_alloc_context3() and avcodec_dct_alloc() respectively, and not >> stored on stack or allocated with av_malloc(sizeof()). > > There is sometimes a disconnect between how an API is meant to be > used, and how 3rd party programs actually use it. In the spirit of > making it "hard(er) to misuse", perhaps the struct definition should be > removed from the public header and instead be written as > > struct AVDCT; > typedef struct AVDCT AVDCT; > > so as to rule out av_malloc(sizeof(AVDCT)). > > >> [[the header file says: >> * You can use AVOptions (av_opt* / av_set/get*()) to access these fields >> from user >> * applications.]] > > A "can" can be read as "need not". And you'd be correct. Direct access is allowed and offsets are guaranteed within a soname version no matter the release. > Perhaps that is what we are seeing with > blender which seems to access av fields directly, and has no noticable > av_set_ calls. > > writeffmpeg.c: if ((of->oformat->flags & AVFMT_GLOBALHEADER)) { > writeffmpeg.c: if (of->oformat->flags & AVFMT_GLOBALHEADER) { > writeffmpeg.c: of->oformat = fmt; > writeffmpeg.c:of->packet_size = rd->ffcodecdata.mux_packet_size; > writeffmpeg.c: of->max_delay = (int)(0.7 * AV_TIME_BASE); > writeffmpeg.c: BLI_strncpy(of->filename, name, sizeof(of->filename)); > writeffmpeg.c:if (avio_open(&of->pb, name, AVIO_FLAG_WRITE) < 0) { > writeffmpeg.c:&of->metadata, context->stamp_data, > ffmpeg_add_metadata_callback, false); > writeffmpeg.c: if (of->pb) { > writeffmpeg.c:avio_close(of->pb); As Thomas mentioned, this looks like valid usage of the API, so the issue in Blender must be something else. All those AVFormatContext fields can be accessed directly. Unrelated to this, but they should stop using of->filename, for that matter, and use of->url instead. The former has been deprecated for two years and a half, and will be removed in an upcoming soname bump. > > Or, summarized: A program may have been built with 4.3 but is combined > with 4.2.3 at runtime, then this can happen: > > a = avcodec_dct_alloc(); // allocates 896 > #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED > a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 > #endif > > If the API should not be used this way, it should not offer this way. But it isn't offered that way. I don't know what part of avdct.h made you think it could be used that way. First, you're setting a pointer you're not meant to set yourself. Nowhere in the doxy it says you can do that. It's not an user settable callback. You're asked to initialize the struct with avcodec_dct_init(). Was it the best design choice? Probably not. Something like pixelutils.h in libavutil may be a better approach, and AVDCT could be changed into something like it. And second, you're running a program built against 4.3 with a lavc 4.2.x at runtime. That is not supported for obvious reasons and the source of your "boom". You can (or should be able to) use 4.3 at runtime on a program built against an ffmpeg release as old as 4.0, but no the opposite way. Your suggestion to change the way we generate the .ver files to outright refuse to run in the above scenario is interesting, but may be quite a lot of work considering the amount of public functions we export. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avfilter/vf_subtitles: add shift option for ass filter
Previously option implemented only for subtitles filter. Signed-off-by: Manolis Stamatogiannakis --- libavfilter/vf_subtitles.c | 35 --- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c index 125fbd9ac7..09cca6aab8 100644 --- a/libavfilter/vf_subtitles.c +++ b/libavfilter/vf_subtitles.c @@ -67,11 +67,12 @@ typedef struct AssContext { #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM #define COMMON_OPTIONS \ -{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ -{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ -{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"filename", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"f", "set the filename of file to read", OFFSET(filename), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"original_size", "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL}, 0, 0, FLAGS }, \ +{"fontsdir", "set the directory containing the fonts to read", OFFSET(fontsdir), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS }, \ +{"alpha", "enable processing of alpha channel", OFFSET(alpha), AV_OPT_TYPE_BOOL, {.i64 = 0 }, 0, 1, FLAGS }, \ +{"shift", "shift subtitles timing", OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0 }, INT64_MIN, INT64_MAX, FLAGS }, \ /* libass supports a log level ranging from 0 to 7 */ static const int ass_libavfilter_log_level_map[] = { @@ -106,7 +107,7 @@ static av_cold int init(AVFilterContext *ctx) if (ass->shift != 0) { ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1, 1000)); -av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n", ass->shift/1000.0); +av_log(ctx, AV_LOG_INFO, "Shifting subtitles by %0.3fsec.\n", ass->shift/1000.0); } ass->library = ass_library_init(); @@ -233,6 +234,8 @@ AVFILTER_DEFINE_CLASS(ass); static av_cold int init_ass(AVFilterContext *ctx) { +int eid, nskip; +ASS_Event *event; AssContext *ass = ctx->priv; int ret = init(ctx); @@ -249,6 +252,24 @@ static av_cold int init_ass(AVFilterContext *ctx) ass->filename); return AVERROR(EINVAL); } + +/* Shift subtitles. */ +nskip = 0; +for (eid = 0; eid < ass->track->n_events; eid++) { +event = &ass->track->events[eid]; +event->Start += ass->shift; +if (event->Start + event->Duration < 0) { +ass_free_event(ass->track, eid); +nskip++; +continue; +} else if (nskip > 0) { +av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time range.\n", nskip); +memmove(event - nskip, event, (ass->track->n_events - eid) * sizeof(ASS_Event)); +ass->track->n_events -= nskip; +nskip = 0; +} +} + return 0; } @@ -273,7 +294,6 @@ static const AVOption subtitles_options[] = { {"stream_index", "set stream index", OFFSET(stream_index), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS}, {"si", "set stream index", OFFSET(stream_index), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS}, {"force_style", "force subtitle style", OFFSET(force_style), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS}, -{"shift","shift subtitles timing", OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS }, {NULL}, }; @@ -466,6 +486,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx) av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n", av_err2str(ret)); } else if (got_subtitle) { +/* Shift subtitles. */ const int64_t start_time = av_rescale_q(sub.pts, AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift;
Re: [FFmpeg-devel] patch submission questions
On Sun, 2020-07-05 at 15:42 +0200, Manolis Stamatogiannakis wrote: > Hello, > > I'm trying to submit a patch for adding a "shift" option to > subtitles/ass > filters. Initial submission was ok, but resubmitting after addressing > some > emails didn't go as expected. > > I have the following two questions on the process: > > Q1: How do you get the "v2" label when you resubmit a patch? I > thought this > would happen auto-magically, but it didn't. > And I couldn't find anything relevant in > https://ffmpeg.org/developer.html#Contributing > > Q2: In a patchset consisting of several commits, is each commit > expected to > be "standalone"? I.e. does it have to apply cleanly without depending > on > the previous commits in the patchset? > Again this is not mentioned in the contributing guidelines, but my > 2/3 > patch didn't make it to patchwork.ffmpeg.org, and this seems like a > plausible explanation. > > Thanks in advance, > Manolis > ___ > 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". You can use `git format-patch -v -n` to get patches like [PATCH v2 1/20]. See git-format-patch documentation for more details. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] patch submission questions
Hello, I'm trying to submit a patch for adding a "shift" option to subtitles/ass filters. Initial submission was ok, but resubmitting after addressing some emails didn't go as expected. I have the following two questions on the process: Q1: How do you get the "v2" label when you resubmit a patch? I thought this would happen auto-magically, but it didn't. And I couldn't find anything relevant in https://ffmpeg.org/developer.html#Contributing Q2: In a patchset consisting of several commits, is each commit expected to be "standalone"? I.e. does it have to apply cleanly without depending on the previous commits in the patchset? Again this is not mentioned in the contributing guidelines, but my 2/3 patch didn't make it to patchwork.ffmpeg.org, and this seems like a plausible explanation. Thanks in advance, Manolis ___ 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] [GSoC 5/6] avformat/utils: add av_packet_clean to remove AVPackets not needed in pktl
- On Jul 5, 2020, at 9:14 PM, Andreas Rheinhardt andreas.rheinha...@gmail.com wrote: > Hongcheng Zhong: >> From: spartazhc >> >> Add av_packet_clean to remove AVPackets whose stream_index is not >> in st_index list. >> >> Generally s->internal->packet_buffer may have pkts from different >> stream, and stream_index will be used to discard pkt that is not >> needed. But in case of abr, several streams may pass the stream_index >> check. So we need a function to remove AVPackets not needed in pktl >> added by hls_read_header. >> >> Signed-off-by: spartazhc >> --- >> libavformat/avformat.h | 9 +++ >> libavformat/internal.h | 15 +++ >> libavformat/utils.c| 59 ++ >> libavformat/version.h | 2 +- >> 4 files changed, 84 insertions(+), 1 deletion(-) >> > > First: I do not know whether we even need this API or not; but I > nevertheless looked at your proposal. > >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h >> index e91e7f1d33..90dee0d075 100644 >> --- a/libavformat/avformat.h >> +++ b/libavformat/avformat.h >> @@ -2474,6 +2474,15 @@ int avformat_seek_file(AVFormatContext *s, int >> stream_index, int64_t min_ts, int >> */ >> int avformat_flush(AVFormatContext *s); >> >> +/** >> + * Remove the AVPackets do not needed in the packet list. >> + * The behaviour is undefined if the packet list is empty. >> + * > > This is completely wrong. It is even more wrong than for > ff_packet_list_clean, as the user has no way to check whether the packet > list is empty. > >> + * @param s media file handle >> + * @param st_index the stream_index list which is needed >> + */ >> +int av_packet_clean(AVFormatContext *s, int *st_index); > > I don't like "clean" as this sounds as if the whole list were discarded; > something like "filter" sounds better. > >> + >> /** >> * Start playing a network-based stream (e.g. RTSP stream) at the >> * current position. >> diff --git a/libavformat/internal.h b/libavformat/internal.h >> index 17a6ab07d3..ac943b1441 100644 >> --- a/libavformat/internal.h >> +++ b/libavformat/internal.h >> @@ -764,6 +764,21 @@ int ff_packet_list_put(AVPacketList **head, AVPacketList >> **tail, >> int ff_packet_list_get(AVPacketList **head, AVPacketList **tail, >> AVPacket *pkt); >> >> +/** >> + * Remove the AVPackets do not needed in the packet list. > > Wrong English "do not needed" > >> + * The behaviour is undefined if the packet list is empty. >> + * > > Undefined if empty? That is very bad behaviour. If it is empty, then > there simply are no packets to discard and this function shouldn't do > anything. > >> + * @note The pkt will be overwritten completely. The caller owns the >> + * packet and must unref it by itself. >> + * > > You obviously copied this (and the undefined if empty) from > ff_packet_list_get. It makes no sense at all here. > >> + * @param head List head element >> + * @param tail List tail element >> + * @param st_index the stream_index list which is needed > > This list needs a length argument (or a documented sentinel). You are > currently hardcoding AVMEDIA_TYPE_NB in an undocumented manner and this > is wrong. > >> + * @return 0 on success. Success is guaranteed >> + * if the packet list is not empty. >> + */ >> +int ff_packet_list_clean(AVPacketList **head, AVPacketList **tail, >> + int *st_index); >> /** >> * Wipe the list and unref all the packets in it. >> * >> diff --git a/libavformat/utils.c b/libavformat/utils.c >> index 45a4179552..26b5a08a19 100644 >> --- a/libavformat/utils.c >> +++ b/libavformat/utils.c >> @@ -1565,6 +1565,65 @@ int ff_packet_list_get(AVPacketList **pkt_buffer, >> return 0; >> } >> >> +/** >> + * return 1 if needed >> + */ >> +static int ff_check_st_index(int st, int *st_index) >> +{ >> +for (int i = 0; i < AVMEDIA_TYPE_NB; ++i) { >> +if (st_index[i] == st) >> +return 1; >> +} >> +return 0; >> +} >> + >> +int ff_packet_list_clean(AVPacketList **pkt_buffer, >> + AVPacketList **pkt_buffer_end, >> + int *st_index) >> +{ >> +AVPacketList *pktl, *pktn; >> +av_assert0(*pkt_buffer); >> +pktl = *pkt_buffer; >> +pktn = pktl->next; >> + >> +/* num >= 3 */ > > ? > >> +while (pktn && pktn != *pkt_buffer_end) { >> +if (!ff_check_st_index(pktn->pkt.stream_index, st_index)) { >> +av_packet_unref(&pktn->pkt); >> +pktl->next = pktn->next; >> +av_freep(pktn); > > This does not free pktn (which leaks -- use valgrind to see it for > yourself). It will instead av_free pktn->pkt.buf which is NULL because > of the av_packet_unref above. > >> +pktn = pktl->next; >> +} else { >> +pktl = pktn; >> +pktn = pktn->next; >> +} >> +} >> +/* last one*/ >> +if (pktn && !ff_check_st_index(p
Re: [FFmpeg-devel] [PATCH] lavc/cfhd:3d transform decoding for both progressive and interlaced
> > > @@ -1064,6 +1446,6 @@ AVCodec ff_cfhd_decoder = { > > .init = cfhd_init, > > .close= cfhd_close, > > .decode = cfhd_decode, > > -.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS, > > -.caps_internal= FF_CODEC_CAP_INIT_THREADSAFE | > FF_CODEC_CAP_INIT_CLEANUP, > > +.capabilities = AV_CODEC_CAP_DR1, > > +.caps_internal= FF_CODEC_CAP_INIT_CLEANUP, > > }; > Dunno, I guess the student just grepped for THREAD and removed it. I did not notice that. Kieran ___ 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] Project orientation
> > So having something merged and maintained in ffmpeg has the benefit that > more people have the ability to test the code and to develop it. Also it > is more likely for the project to get developers if their code live in the > core ffmpeg respository. I also don't see that maitenance burden to be so > huge. And usefulness is also questionable. I think it is safe to say that > having AMQP/ZMQ protocol support is much more useful then Lego Racers > demuxer... (and I quite liked Lego Racers!!!). > Maybe also a full implementation of WebRTC? Or QUIC? The problem is that libavformat is not a network stack and adding more protocols means that it's harder and harder to build a real network stack. See the way UDP has a hacky thread built in to stop packet loss. Kieran ___ 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] Project orientation
> > > Would you want something experimental like x262 to be part of the > > FFmpeg codebase, for us to have to maintain forever? If you don't limit > > scope then that is what would happen. > > x262 is another example of a fork, where the fork alone was not > popular/interesting enough to live on. If it were merged to x264, I am > fairly certain it would not be experimental anymore, and we'd have an > MPEG2 encoder which would scale much better to multiple cores than what we > have now in ffmpeg. > Highly unlikely, x264 development has essentially ground to a halt. And people use H.264 a lot still. x262 works well enough for an old format like MPEG-2. There's no real need to develop it unless someone needs to eke out an extra 2% compression because they have a million MPEG-2 receivers they can't change. The original x264 developers didn't want a merge back by the way. And I'd also like to point to the linux kernel as an example of a > monolitic code repository which seems to work quite well. > Exception that proves the rule. A lot of developers there are paid full time to work on the kernel and just do cleanup, merge old patches etc. Kieran ___ 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] [GSoC 5/6] avformat/utils: add av_packet_clean to remove AVPackets not needed in pktl
Hongcheng Zhong: > From: spartazhc > > Add av_packet_clean to remove AVPackets whose stream_index is not > in st_index list. > > Generally s->internal->packet_buffer may have pkts from different > stream, and stream_index will be used to discard pkt that is not > needed. But in case of abr, several streams may pass the stream_index > check. So we need a function to remove AVPackets not needed in pktl > added by hls_read_header. > > Signed-off-by: spartazhc > --- > libavformat/avformat.h | 9 +++ > libavformat/internal.h | 15 +++ > libavformat/utils.c| 59 ++ > libavformat/version.h | 2 +- > 4 files changed, 84 insertions(+), 1 deletion(-) > First: I do not know whether we even need this API or not; but I nevertheless looked at your proposal. > diff --git a/libavformat/avformat.h b/libavformat/avformat.h > index e91e7f1d33..90dee0d075 100644 > --- a/libavformat/avformat.h > +++ b/libavformat/avformat.h > @@ -2474,6 +2474,15 @@ int avformat_seek_file(AVFormatContext *s, int > stream_index, int64_t min_ts, int > */ > int avformat_flush(AVFormatContext *s); > > +/** > + * Remove the AVPackets do not needed in the packet list. > + * The behaviour is undefined if the packet list is empty. > + * This is completely wrong. It is even more wrong than for ff_packet_list_clean, as the user has no way to check whether the packet list is empty. > + * @param s media file handle > + * @param st_index the stream_index list which is needed > + */ > +int av_packet_clean(AVFormatContext *s, int *st_index); I don't like "clean" as this sounds as if the whole list were discarded; something like "filter" sounds better. > + > /** > * Start playing a network-based stream (e.g. RTSP stream) at the > * current position. > diff --git a/libavformat/internal.h b/libavformat/internal.h > index 17a6ab07d3..ac943b1441 100644 > --- a/libavformat/internal.h > +++ b/libavformat/internal.h > @@ -764,6 +764,21 @@ int ff_packet_list_put(AVPacketList **head, AVPacketList > **tail, > int ff_packet_list_get(AVPacketList **head, AVPacketList **tail, > AVPacket *pkt); > > +/** > + * Remove the AVPackets do not needed in the packet list. Wrong English "do not needed" > + * The behaviour is undefined if the packet list is empty. > + * Undefined if empty? That is very bad behaviour. If it is empty, then there simply are no packets to discard and this function shouldn't do anything. > + * @note The pkt will be overwritten completely. The caller owns the > + * packet and must unref it by itself. > + * You obviously copied this (and the undefined if empty) from ff_packet_list_get. It makes no sense at all here. > + * @param head List head element > + * @param tail List tail element > + * @param st_index the stream_index list which is needed This list needs a length argument (or a documented sentinel). You are currently hardcoding AVMEDIA_TYPE_NB in an undocumented manner and this is wrong. > + * @return 0 on success. Success is guaranteed > + * if the packet list is not empty. > + */ > +int ff_packet_list_clean(AVPacketList **head, AVPacketList **tail, > + int *st_index); > /** > * Wipe the list and unref all the packets in it. > * > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 45a4179552..26b5a08a19 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -1565,6 +1565,65 @@ int ff_packet_list_get(AVPacketList **pkt_buffer, > return 0; > } > > +/** > + * return 1 if needed > + */ > +static int ff_check_st_index(int st, int *st_index) > +{ > +for (int i = 0; i < AVMEDIA_TYPE_NB; ++i) { > +if (st_index[i] == st) > +return 1; > +} > +return 0; > +} > + > +int ff_packet_list_clean(AVPacketList **pkt_buffer, > + AVPacketList **pkt_buffer_end, > + int *st_index) > +{ > +AVPacketList *pktl, *pktn; > +av_assert0(*pkt_buffer); > +pktl = *pkt_buffer; > +pktn = pktl->next; > + > +/* num >= 3 */ ? > +while (pktn && pktn != *pkt_buffer_end) { > +if (!ff_check_st_index(pktn->pkt.stream_index, st_index)) { > +av_packet_unref(&pktn->pkt); > +pktl->next = pktn->next; > +av_freep(pktn); This does not free pktn (which leaks -- use valgrind to see it for yourself). It will instead av_free pktn->pkt.buf which is NULL because of the av_packet_unref above. > +pktn = pktl->next; > +} else { > +pktl = pktn; > +pktn = pktn->next; > +} > +} > +/* last one*/ > +if (pktn && !ff_check_st_index(pktn->pkt.stream_index, st_index)) { > +av_packet_unref(&pktn->pkt); > +av_freep(pktn); > +pktl->next = NULL; > +*pkt_buffer_end = pktl; > +} Is handling the last one differently really necessary? Wouldn't it be enough to remove pktn != *pkt
[FFmpeg-devel] [PATCH 3/3] avfilter/vf_subtitles: Reorganized subtitles filter options.
Some options are common between subtitles/ass filters. Rather than mentioning for each option whether it is common or not, the options are now displayed in two separate tables. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 18 +++--- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index c962ac55b0..c4ca39cb6d 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -6453,6 +6453,7 @@ This filter supports the following @ref{commands} that corresponds to option of @item planes @end table +@anchor{ass} @section ass Same as the @ref{subtitles} filter, except that it doesn't require libavcodec @@ -17929,15 +17930,12 @@ To enable compilation of this filter you need to configure FFmpeg with libavformat to convert the passed subtitles file to ASS (Advanced Substation Alpha) subtitles format. -The filter accepts the following options: +Common @ref{subtitles}/@ref{ass} filter options: @table @option @item filename, f Set the filename of the subtitle file to read. It must be specified. -@item shift -Shift subtitles timings by the specified amount. - @item original_size Specify the size of the original video, the video for which the ASS file was composed. For the syntax of this option, check the @@ -17952,12 +17950,18 @@ These fonts will be used in addition to whatever the font provider uses. @item alpha Process alpha channel, by default alpha channel is untouched. +@item shift +Shift subtitles timings by the specified amount. +@end table + +Additional options for @ref{subtitles} filter: + +@table @option @item charenc -Set subtitles input character encoding. @code{subtitles} filter only. Only -useful if not UTF-8. +Set subtitles input character encoding. Only useful if not UTF-8. @item stream_index, si -Set subtitles stream index. @code{subtitles} filter only. +Set subtitles stream index. @item force_style Override default style or script info parameters of the subtitles. It accepts a -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] avfilter/vf_subtitles: add shift option
Allows shifting of subtitle display times to align them with the video. This avoids having to rewrite the subtitle file in order to display subtitles correctly when input is seeked (-ss). Also handy for minor subtitle timing corrections without rewriting the subtitles file. Signed-off-by: Manolis Stamatogiannakis --- doc/filters.texi | 8 libavfilter/vf_subtitles.c | 23 +-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index ad2448acb2..c962ac55b0 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -17935,6 +17935,9 @@ The filter accepts the following options: @item filename, f Set the filename of the subtitle file to read. It must be specified. +@item shift +Shift subtitles timings by the specified amount. + @item original_size Specify the size of the original video, the video for which the ASS file was composed. For the syntax of this option, check the @@ -17991,6 +17994,11 @@ To make the subtitles stream from @file{sub.srt} appear in 80% transparent blue subtitles=sub.srt:force_style='Fontname=DejaVu Serif,PrimaryColour=&HCCFF' @end example +To re-sync subtitles after seeking the input e.g. with @code{-ss 20:20}, use: +@example +subtitles=filename=sub.srt:shift='-20\:20' +@end example + @section super2xsai Scale the input by 2x and smooth using the Super2xSaI (Scale and diff --git a/libavfilter/vf_subtitles.c b/libavfilter/vf_subtitles.c index 1bd42391e0..125fbd9ac7 100644 --- a/libavfilter/vf_subtitles.c +++ b/libavfilter/vf_subtitles.c @@ -52,6 +52,7 @@ typedef struct AssContext { char *filename; char *fontsdir; char *charenc; +int64_t shift; char *force_style; int stream_index; int alpha; @@ -103,6 +104,11 @@ static av_cold int init(AVFilterContext *ctx) return AVERROR(EINVAL); } +if (ass->shift != 0) { +ass->shift = av_rescale_q(ass->shift, AV_TIME_BASE_Q, av_make_q(1, 1000)); +av_log(ctx, AV_LOG_DEBUG, "Shifting subtitles by %0.3fsec.\n", ass->shift/1000.0); +} + ass->library = ass_library_init(); if (!ass->library) { av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n"); @@ -267,6 +273,7 @@ static const AVOption subtitles_options[] = { {"stream_index", "set stream index", OFFSET(stream_index), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS}, {"si", "set stream index", OFFSET(stream_index), AV_OPT_TYPE_INT,{ .i64 = -1 }, -1, INT_MAX, FLAGS}, {"force_style", "force subtitle style", OFFSET(force_style), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, FLAGS}, +{"shift","shift subtitles timing", OFFSET(shift), AV_OPT_TYPE_DURATION, {.i64 = 0}, INT64_MIN, INT64_MAX, FLAGS }, {NULL}, }; @@ -297,7 +304,7 @@ AVFILTER_DEFINE_CLASS(subtitles); static av_cold int init_subtitles(AVFilterContext *ctx) { -int j, ret, sid; +int j, ret, sid, nskip; int k = 0; AVDictionary *codec_opts = NULL; AVFormatContext *fmt = NULL; @@ -448,6 +455,7 @@ static av_cold int init_subtitles(AVFilterContext *ctx) av_init_packet(&pkt); pkt.data = NULL; pkt.size = 0; +nskip = 0; while (av_read_frame(fmt, &pkt) >= 0) { int i, got_subtitle; AVSubtitle sub = {0}; @@ -458,8 +466,17 @@ static av_cold int init_subtitles(AVFilterContext *ctx) av_log(ctx, AV_LOG_WARNING, "Error decoding: %s (ignored)\n", av_err2str(ret)); } else if (got_subtitle) { -const int64_t start_time = av_rescale_q(sub.pts, AV_TIME_BASE_Q, av_make_q(1, 1000)); +const int64_t start_time = av_rescale_q(sub.pts, AV_TIME_BASE_Q, av_make_q(1, 1000)) + ass->shift; const int64_t duration = sub.end_display_time; + +if (start_time + duration < 0) { +nskip++; +goto pkt_end; +} else if (nskip > 0) { +av_log(ctx, AV_LOG_INFO, "Skipped %d subtitles out of time range.\n", nskip); +nskip = 0; +} + for (i = 0; i < sub.num_rects; i++) { char *ass_line = sub.rects[i]->ass; if (!ass_line) @@ -472,6 +489,8 @@ static av_cold int init_subtitles(AVFilterContext *ctx) } } } + +pkt_end: av_packet_unref(&pkt); avsubtitle_free(&sub); } -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Project orientation
On Sun, 5 Jul 2020, Tomas Härdin wrote: sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: On Sun, 5 Jul 2020, Tomas Härdin wrote: > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > wrote: > > > [...] > > > > At some point, the project needs to decide what is in and what is > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > externally. It's not a problem to say "No" to a feature, > > > > especially when, the number of people using that feature is under > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > anyway. > > > > > > what we need is not to say "no" but to say > > > "use this API, implement the module in your own repository, and > > > register your module there" > > > > Once again, Michael, we agree, on this topic. > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > useful". > > I'd like to express a general agreement with this point. The project > has experienced quite a lot of scope creep lately. We don't have to > accommodate everyone, especially with the finite number of developers > available. > > We can encourage people to maintain their own forks of FFmpeg, see for > example FFmbc. And see how dead that project is. Or ffserver. Or x262. That may be because Baptiste is focusing on other things. The code is still there. It still compiles. That is the point. Baptise is focusing on other things, and the code he used to do rots in an external repository. I guess most of the features he implemented crept back to ffmpeg (I did the backporting of some features myself), but still, it was duplicate work. Forks are good, if you are submitting the code back to master. If not, then forks are not so good because they often dont't live on alone and you lose the features in it. Would you want something experimental like x262 to be part of the FFmpeg codebase, for us to have to maintain forever? If you don't limit scope then that is what would happen. x262 is another example of a fork, where the fork alone was not popular/interesting enough to live on. If it were merged to x264, I am fairly certain it would not be experimental anymore, and we'd have an MPEG2 encoder which would scale much better to multiple cores than what we have now in ffmpeg. So having something merged and maintained in ffmpeg has the benefit that more people have the ability to test the code and to develop it. Also it is more likely for the project to get developers if their code live in the core ffmpeg respository. I also don't see that maitenance burden to be so huge. And usefulness is also questionable. I think it is safe to say that having AMQP/ZMQ protocol support is much more useful then Lego Racers demuxer... (and I quite liked Lego Racers!!!). So my opinion would be to be inclusive when merging stuff. I am not saying everything has to be merged, I rejected not very long ago the NIT table parsing or the EPG "data decoder", these were really out of scope and more importantly out of the current capabilites of the framework. And if distros are worried about dependencies, they can always make two packages, a normal and a full. And I'd also like to point to the linux kernel as an example of a monolitic code repository which seems to work quite well. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavformat/hlsenc: Remove duplicate close of the output stream.
Ping? On 2020-07-01 17:59, Andrey Semashev wrote: The result of the first close attempt is ignored and may be lost. By removing it we ensure the close result code is properly analyzed. --- libavformat/hlsenc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 71fa3db060..88b58a1ba8 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -2631,7 +2631,6 @@ static int hls_write_trailer(struct AVFormatContext *s) goto failed; vs->size = range_length; -hlsenc_io_close(s, &vs->out, filename); ret = hlsenc_io_close(s, &vs->out, filename); if (ret < 0) { av_log(s, AV_LOG_WARNING, "upload segment failed, will retry with a new http session.\n"); ___ 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] ABI break in 4.3
On Sunday 2020-07-05 13:39, Tomas Härdin wrote: >>> Downgrading to a .so file with a lower minor version number than >>> the program is built against can never be expected to work. Else >>> we couldn't add new functions without a major bump. >> >> It requires the use ELF symbol versions -- which ffmpeg fails to >> do properly.[...] > >This is a fair point. I didn't actually know the loader can do stuff >like this, sounds super handy. How hard would it be to get that going? Change libavcodec.v to LIBAVCODEC_58 { global: av_foo; av_bar; av_whatever_else_there_is;... local: *; }; LIBAVCODEC_58.91 { global: avpriv_mpeg4audio_get_config2; } LIBAVCODEC_58; LIBAVCODEC_58.123 { /* future example */ global: avblahblah; } LIBAVCODEC_58.91; Repeat likewise for other .v files. The file is no longer a template. The automatic substitution of "_MAJOR" by the build system needs to cease. Version numbers in the file are supposed to be fixed (among the set of all .so files that share a SONAME). ___ 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 v06 2/5] fbtile helperRoutines cpu based framebuffer detiling
Jul 5, 2020, 04:57 by hanish...@gmail.com: > Hi Lynne, > > > On Sun, 5 Jul, 2020, 00:53 Lynne, wrote: > >> Jul 4, 2020, 14:17 by hanish...@gmail.com: >> >> > Add helper routines which can be used to detile tiled framebuffer >> > layouts into a linear layout, using the cpu. >> > >> > Currently it supports Legacy Intel Tile-X, Legacy Intel Tile-Y and >> > Newer Intel Tile-Yf tiled layouts. >> > >> > Currently supported pixel format is 32bit RGB. >> > >> > It also contains detile_generic logic, which can be easily configured >> > to support different kinds of tiling layouts, at the expense of some >> > processing speed, compared to developing a targeted detiling logic. >> > >> >> Sorry, but we really cannot afford to have this as part of the public API, >> which it needs to be if it can be used by lavfi. So its unacceptable in its >> current form, especially if all parts dependent on it are compile-time >> options. >> That's why I suggested merging it all into hwcontext_drm.c >> > > > > There is no compile time options which it's users/callers need to > manipulate to use it. > > What I meant by easily configurable is, in future if we want to add support > for a new (currently unknown to it) tile layout, there is no need to do it > from scratch. One could just add a new set of boundry conditions with the > required direction changes, and the generic logic can take care of doing > the required tile walking. This is no different than say adding a new > colour format or bit resolution or so to a scaler for example, rather I > would say much simpler than these. > > And once the new definition is added into fbtile. It can be used by any of > its users without any additional change. > > That's how any library or for that matter any code works currently, so not > sure what's diffrent here that you are objecting to. > > Also tile manipulation is a logically independent set of operations which > can be used by any logic and not just hwcontext_drm, and tiling is nothing > unique to hwcontext_drm, so it will be better to keep it independent of > hwcontext_drm, because logically/technically it's independent of it. > > Also I seem to be missing something here, what is the technical/... reason > you don't want a frame buffer tile manipulation logic as a public API, in > case if it is? > Something like this, is like I said, very niche and optionally fixes behavior that shouldn't ever have been exposed had it not been for the incredibly bad decisions libdrm made (and we made as well). And for that, and for not fitting in as any part of any library, I don't want a public API for this anywhere. Something like this could belong in libswscale, but that requires not resorting to hacks and actually implementing tiled pixel formats, which we're definitely not doing. Hence any patches that actually fix the behavior non-optionally would be what's accepted. We're not looking for solutions outside of hwcontext_drm. Like I said, no other API is that badly designed to give the user access to tiled data. There' no point in having a generic filter/hwdownload setting because there's nothing other than hwcontext_drm that would use it properly. While I'm okay with having the detiling as a separate file, it cannot be exposed via the public API and any user wishing to detile would have to use the hwcontext API. And I don't want this as an optional filter either. And no, you can't just duplicate the tiling code to libavfilter. Bottom line is, we don't want command-line users to _ever_ see or touch tiled content, and to do that, you have to make hwcontext_drm.c detile when downloading. ___ 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] Project orientation
Quoting Tomas Härdin (2020-07-05 13:19:25) > sön 2020-07-05 klockan 12:42 +0200 skrev Marton Balint: > > > > On Sun, 5 Jul 2020, Tomas Härdin wrote: > > > > > sön 2020-07-05 klockan 00:10 +0200 skrev Jean-Baptiste Kempf: > > > > On Sat, Jul 4, 2020, at 23:51, Michael Niedermayer wrote: > > > > > On Sat, Jul 04, 2020 at 11:25:31PM +0200, Jean-Baptiste Kempf > > > > > wrote: > > > > > [...] > > > > > > At some point, the project needs to decide what is in and what is > > > > > > out, and since FFmpeg has numerous APIs, people can plug them > > > > > > externally. It's not a problem to say "No" to a feature, > > > > > > especially when, the number of people using that feature is under > > > > > > 0,01% of FFmpeg users, and when people can build that externally > > > > > > anyway. > > > > > > > > > > what we need is not to say "no" but to say > > > > > "use this API, implement the module in your own repository, and > > > > > register your module there" > > > > > > > > Once again, Michael, we agree, on this topic. > > > > > > > > No, means "not in the ffmpeg repo" does not mean "no, this is not > > > > useful". > > > > > > I'd like to express a general agreement with this point. The project > > > has experienced quite a lot of scope creep lately. We don't have to > > > accommodate everyone, especially with the finite number of developers > > > available. > > > > > > We can encourage people to maintain their own forks of FFmpeg, see for > > > example FFmbc. > > > > And see how dead that project is. Or ffserver. Or x262. > > That may be because Baptiste is focusing on other things. The code is > still there. It still compiles. > > Would you want something experimental like x262 to be part of the > FFmpeg codebase, for us to have to maintain forever? If you don't limit > scope then that is what would happen. I have been thinking about something like a staging branch. Or multiple such "experimental" branches in the main repository, which would still be "official" in some sense, even though they wouldn't be the master one. There obviously seems to be a conflict of visions between people who want ffmpeg to be a stable consistent coherent tool for our downstreams and those who want to play with experimental concepts, cute hacks and fringe features. I would count myself among the former, but the latter is also a valid worldview and should not be disregarded. I would like to hope some kind of a compromise can be found. Will write a more complete reply to OP later. -- Anton Khirnov ___ 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] ABI break in 4.3
sön 2020-07-05 klockan 12:54 +0200 skrev Jan Engelhardt: > On Sunday 2020-07-05 12:06, Tomas Härdin wrote: > > > Or, summarized: A program may have been built with 4.3 but is > > > combined > > > with 4.2.3 at runtime, then this can happen: > > > > > > a = avcodec_dct_alloc(); // allocates 896 > > > #ifdef HAVE_STRUCT_AVDCT_GET_PIXELS_UNALIGNED > > > a->get_pixels_unaligned = ffunc; // boom accessing byte ~952 > > > #endif > > > > "Doctor it hurts when I do this!" > > The application of this saying is short-sighted. It confounds basic > exercise of features with overuse of said features. For lack of a > better analogy, moving a leg normally _ought_ not to hurt in healthy > humans. What is seen as "normal" is indeed situation-dependent, but > the only other way to look at it is that ffmpeg is a disabled entity > with special needs. We can certainly change the API to make this sort of break harder, with a major bump. But given what the documentation says about the current API it is not a break. It might not be the most user-friendly API, but it is right there in the headers. There's quite a bit of inertia involved too. This makes me wonder if there's a way to prevent AVCodecContext from being allocated anywhere but the heap, without having it and similar structs just be opaque pointers on the user side. > > Downgrading to a .so file with a lower minor version number than > > the > > program is built against can never be expected to work. Else we > > couldn't add new functions without a major bump. > > Then you are doing it wrong. If one tries to run a contemporary > program on an older distribution, which certainly has an older glibc, > it refuses to run - which is much preferable to a crash at an > arbitrary point down the line. > > It requires the use ELF symbol versions -- which ffmpeg fails to > do properly. Between 4.2.3 and 4.3, > > avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58 > > which is wrong. It should have been > > avpriv_mpeg4audio_get_config2@@LIBAVCODEC_58.91 > > Then the runtime linker ld-linux.so would have caught the problem > at startup, because then, a program built with 4.3 would have a > minimum requirement on elfsymver "58.91", and *not just* "58". This is a fair point. I didn't actually know the loader can do stuff like this, sounds super handy. How hard would it be to get that going? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".