Re: [FFmpeg-devel] RFC: new packed pixel formats (machine vision)
On Wed, Oct 23, 2024 at 2:04 AM martin schitter wrote: > > > > On 22.10.24 22:33, Diederick C. Niehorster wrote: > > I am writing about machine vision, not machine learning or computer > > vision. So there are no uncommon small bit sizes, we're dealing with > > 8bit, 10bit, 12bit components here. > > Sorry -- I'm such a sloppy reader/writer -- especially, when I'm hurry. > > > Where possible, i already map to the matching ffmpeg format, the > > problem i am running into is that there isn't one for some of the > > common machine vision pixel formats. > > While this can be fixed with an encoder, that would complicate their > > use in ffmpeg. Having them instead as pixel formats supported by > > swscale as inputs make them far more generally useful, and enables > > easy passing these formats to many of ffmpeg's encoders using an > > auto-negotiated/inserted scale filter. > > I'm not a big fan of these auto-negotiated format reading, because the > actual code which handles this task looks utterly unreadable to me > --full of exceptions and complicated switches of code flow, which also > hinders a more efficient processing. > > But o.k. it is a comfortable and simple solution for simple demands and > may even help to reduce bugs, which would otherwise more frequently > appear by writing new more application/format specific handlers. > > > In the previous discussion, Lynne also indicated that the inclusion of > > such formats is in scope for ffmpeg, as there are also cinema cameras > > that produce some of them. > > Yes he pointed to this already available Bayer Format entries. > > They obviously work different from other pixel format description > entries. I still don't really grasp, how they work exactly. > > CFA sensor data is usually more structured like a one channel pixel > matrix similar to a monochrome image. The colors are only later > calculated in the debayer process. But at the beginning you have only > this one-channel matrix of values and an additional description of the > used CFA arrangement -- i.e. the location of the different colored > sensel in relation to each other. > > This colored graphics in your linked documents are therefore a little > bit misleading, because if you really would differentiate the colored > sensels already at this stage, you would have describe different data > patterns for odd an even lines in case of a typical image sensor... > > >>>> Example formats are 10 and 12 bit Bayer formats, where the 10 bit > >>>> cannot be represented in AVPixFmtDescriptors as currently as effective > >>>> bit depth for the red and blue channels is 2.5 bits, but component > >>>> depths should be integers. > > At least in case of all ordinary pixel arrangement description entries > the values are not just useful metadata for further calculations, but > real descriptions, where to find the actual data -- i.e.: which > bytes/bits to pick out of the raw data stream. > > >> As bits will always be distinct entities, you don't need more than > >> simple natural numbers to describe their placement and amount precisely. > > > An AVPixFmtDescriptor encodes the effective number of bits. Here the > > descriptor for 8 bit bayer formats already included with ffmpeg: > > #define BAYER8_DESC_COMMON \ > > .nb_components= 3, \ > > .log2_chroma_w= 0, \ > > .log2_chroma_h= 0, \ > > .comp = { \ > > { 0, 1, 0, 0, 2 }, \ > > { 0, 1, 0, 0, 4 }, \ > > { 0, 1, 0, 0, 2 }, \ > > } > > Note that the green component is denoted as having 4 bits, and the red > > and blue as 2 bits. That is because there are only one blue and red > > sample per 4 pixels, and one per 2 pixels for green samples, leading > > to _effective bitdepths_ of 8/4=2 for red and blue, and 8/2=4 for > > green. > > It definition so much different to the more ordinary pixel descriptions, > that hardly understand why they are mixed together at all? > > An additional list with more RAW/CFA specific description fields, would > IMHO be a much more suitable solution. > > >> ffmpeg already supports the AV_PIX_FMT_FLAG_BITSTREAM to switch some > >> description fields from byte to bit values. That's enough to describe > >> the layout of most pixelformats -- even those packed ones, which are not > >> aligned to byte or 32bit borders. You just have to use bit size values > >> for step and offset stuct members. > > > > Lynne indicated that AV_PIX_FMT_FLAG_BITSTREAM is only
Re: [FFmpeg-devel] [PATCH v12 5/9] libavcodec/dnxucdec: DNxUncompressed decoder
On Wed, Oct 23, 2024 at 8:44 AM James Almer wrote: > > On 10/21/2024 4:57 PM, Martin Schitter wrote: > > --- > > libavcodec/Makefile| 1 + > > libavcodec/allcodecs.c | 1 + > > libavcodec/dnxucdec.c | 338 + > > 3 files changed, 340 insertions(+) > > create mode 100644 libavcodec/dnxucdec.c > > > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > > index dd5d0de..e13b127 100644 > > --- a/libavcodec/Makefile > > +++ b/libavcodec/Makefile > > @@ -328,6 +328,7 @@ OBJS-$(CONFIG_DFPWM_DECODER) += dfpwmdec.o > > OBJS-$(CONFIG_DFPWM_ENCODER) += dfpwmenc.o > > OBJS-$(CONFIG_DNXHD_DECODER) += dnxhddec.o dnxhddata.o > > OBJS-$(CONFIG_DNXHD_ENCODER) += dnxhdenc.o dnxhddata.o > > +OBJS-$(CONFIG_DNXUC_DECODER) += dnxucdec.o > > OBJS-$(CONFIG_DOLBY_E_DECODER) += dolby_e.o dolby_e_parse.o > > kbdwin.o > > OBJS-$(CONFIG_DPX_DECODER) += dpx.o > > OBJS-$(CONFIG_DPX_ENCODER) += dpxenc.o > > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > > index c7e5f99..ccca2ad 100644 > > --- a/libavcodec/allcodecs.c > > +++ b/libavcodec/allcodecs.c > > @@ -93,6 +93,7 @@ extern const FFCodec ff_dfa_decoder; > > extern const FFCodec ff_dirac_decoder; > > extern const FFCodec ff_dnxhd_encoder; > > extern const FFCodec ff_dnxhd_decoder; > > +extern const FFCodec ff_dnxuc_decoder; > > extern const FFCodec ff_dpx_encoder; > > extern const FFCodec ff_dpx_decoder; > > extern const FFCodec ff_dsicinvideo_decoder; > > diff --git a/libavcodec/dnxucdec.c b/libavcodec/dnxucdec.c > > new file mode 100644 > > index 000..9d5847d > > --- /dev/null > > +++ b/libavcodec/dnxucdec.c > > @@ -0,0 +1,338 @@ > > +/* > > + * Avid DNxUncomressed / SMPTE RDD 50 decoder > > + * Copyright (c) 2024 Martin Schitter > > + * > > + * This file is part of FFmpeg. > > + * > > + * FFmpeg is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU Lesser General Public > > + * License as published by the Free Software Foundation; either > > + * version 2.1 of the License, or (at your option) any later version. > > + * > > + * FFmpeg is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + * Lesser General Public License for more details. > > + * > > + * You should have received a copy of the GNU Lesser General Public > > + * License along with FFmpeg; if not, write to the Free Software > > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA > > 02110-1301 USA > > + */ > > + > > +/* > > + This decoder for DNxUncompressed video data is mostly based on > > + reverse engineering of output generated by DaVinci Resolve 19 > > + but was later also checked against the SMPTE RDD 50 specification. > > + > > + Not all DNxUncompressed pixel format variants are supported, > > + but at least an elementary base set is already usable: > > + > > + - YUV 4:2:2 8/10/12/16bit/half/float (16bit untested) > > +YUV 4:4:4 8/16bit/half/float (all untested!) > > + - RGB 8/10/12/16bit/half/float (16bit untested) > > +Alpha/Y 8/16bit (all untested!) > > That's not good... > > [...] Is your suggested to only include the cases that have been tested and remove the rest? That would make sense, it seems other cases can be added trivially once samples become available now that this groundwork is done. @Martin, is there any way you could get samples of the other formats? Perhaps by downloading a trial version of acid media composer, or if my google fu works well, by installing a codec pack https://kb.avid.com/pkb/articles/en_US/Knowledge/Avid-QuickTime-Codecs-LE,w hich should allow using these codecs in after effects and such. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] RFC: new packed pixel formats (machine vision)
Hi Martin, Thanks for writing in! On Tue, Oct 22, 2024 at 11:41 AM martin schitter wrote: > > > > On 22.10.24 08:50, Diederick C. Niehorster wrote: > >> I want to pick up a discussion i started last week > >> (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-October/334585.html) > >> in a new thread, with the relevant information nicely organized. This > >> is about adding pixel formats common in machine vision to ffmpeg > >> (though i understand some formats may also be used by cinema cameras), > >> and supporting them as input formats in swscale so that it becomes > >> easy to use ffmpeg for machine vision purposes (I already have such > >> software, it will be open-sourced in good time, but right now there is > >> a proprietary conversion layer from Basler i need to replace (e.g. by > >> this proposal)). > > most of your point do not look so much machine learning or computer > vision specific, but more like typical/traditional video tech > peculiarities. More ML related obstacles come into play, if have to > support optimized calculations with uncommon small bit sizes, etc. But > most of your described issues should be solvable easily by already > available features of ffmpeg, if I'm not wrong. I am writing about machine vision, not machine learning or computer vision. So there are no uncommon small bit sizes, we're dealing with 8bit, 10bit, 12bit components here. Where possible, i already map to the matching ffmpeg format, the problem i am running into is that there isn't one for some of the common machine vision pixel formats. While this can be fixed with an encoder, that would complicate their use in ffmpeg. Having them instead as pixel formats supported by swscale as inputs make them far more generally useful, and enables easy passing these formats to many of ffmpeg's encoders using an auto-negotiated/inserted scale filter. In the previous discussion, Lynne also indicated that the inclusion of such formats is in scope for ffmpeg, as there are also cinema cameras that produce some of them. > >> Example formats are 10 and 12 bit Bayer formats, where the 10 bit > >> cannot be represented in AVPixFmtDescriptors as currently as effective > >> bit depth for the red and blue channels is 2.5 bits, but component > >> depths should be integers. > > As bits will always be distinct entities, you don't need more than > simple natural numbers to describe their placement and amount precisely. An AVPixFmtDescriptor encodes the effective number of bits. Here the descriptor for 8 bit bayer formats already included with ffmpeg: #define BAYER8_DESC_COMMON \ .nb_components= 3, \ .log2_chroma_w= 0, \ .log2_chroma_h= 0, \ .comp = { \ { 0, 1, 0, 0, 2 }, \ { 0, 1, 0, 0, 4 }, \ { 0, 1, 0, 0, 2 }, \ } Note that the green component is denoted as having 4 bits, and the red and blue as 2 bits. That is because there are only one blue and red sample per 4 pixels, and one per 2 pixels for green samples, leading to _effective bitdepths_ of 8/4=2 for red and blue, and 8/2=4 for green. For 10bit bayer, this leads to having 2.5 effective bits for red and blue. Hence the proposal i made. > ffmpeg already supports the AV_PIX_FMT_FLAG_BITSTREAM to switch some > description fields from byte to bit values. That's enough to describe > the layout of most pixelformats -- even those packed ones, which are not > aligned to byte or 32bit borders. You just have to use bit size values > for step and offset stuct members. Lynne indicated that AV_PIX_FMT_FLAG_BITSTREAM is only for 8bit and 32bit aligned formats. Here i'm dealing with unaligned formats. An option could be to release the restriction that AV_PIX_FMT_FLAG_BITSTREAM needs to be 8bit or 32bit aligned, but that would be a backwards incompatible change with not only significant repercussions for the ffmpeg codebase, but also for user code. It is better to have a new flag for the new situation. > But there is another common case, which is indeed not describable with > ffmpeg current stuct: color components can be composed out of separated > MSb and LSb parts at different places in the component sequenz -- > similar to the color examples BayerRG12g40 and BayerRG12g24 in your > linked examples. Although these examples are indeed a little bit more > complex, because they may describe arrangements, which differ between > even and odd lanes. The bit packing for 10 and 12bit data in > DNxUncompressed entails a similar issue, by packing all LSb information > as one block at the end of every scan line. I think these are less common, the one exception being some Gige cameras that spread the msb of multiple components after 8 lsb bits for each. I think these are suffic
Re: [FFmpeg-devel] RFC: new packed pixel formats (machine vision)
Ping. Is the below a viable scheme or are there concerns to consider in this initial design stage? Thanks and all the best, Dee On Tue, Oct 15, 2024 at 8:55 AM Diederick C. Niehorster wrote: > > Hi All, > > I want to pick up a discussion i started last week > (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-October/334585.html) > in a new thread, with the relevant information nicely organized. This > is about adding pixel formats common in machine vision to ffmpeg > (though i understand some formats may also be used by cinema cameras), > and supporting them as input formats in swscale so that it becomes > easy to use ffmpeg for machine vision purposes (I already have such > software, it will be open-sourced in good time, but right now there is > a proprietary conversion layer from Basler i need to replace (e.g. by > this proposal)). > > Example formats are 10 and 12 bit Bayer formats, where the 10 bit > cannot be represented in AVPixFmtDescriptors as currently as effective > bit depth for the red and blue channels is 2.5 bits, but component > depths should be integers. Other example formats are 10bit gray > formats where multiple values are packed without padding over multiple > bytes (e.g. 4 10-bit pixels packed into 5 bytes, so not aligned to 16 > or 32 bits). > > See > https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-monochrome-pixel-formats.html > for a diagram of the Mono10p and > https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-raw-bayer-pixel-formats.html > for diagrams of the packed and not packed bayer formats. > > Here a proposal for how these new formats could be encoded into > AVPixFmtDescriptor, so that these can then be used in ffmpeg/swscale. > I have taken care that none of the existing pixel formats or any code > dealing with them would be affected, although new code would be needed > to handle these new formats (av_read_image_line2, av_write_image_line2 > and functions printing info about AVPixFmtDescriptors, plus swscale of > course--i commit to do a full audit to ensure nothing else is missed). > > First, two new flags are needed (usages are shown below in the example > new pixel formats). I propose: > - AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL which indicates that the value > in the component depths (ints) represent a 16 bit numerator and > denominator packed into the int. That should be able to store any > value that could ever be possible and importantly allows for the > fractional bit depths needed for the bayer formats. > - AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED which indicates formats that are > bit-wise packed in a way that is not aligned on 1, 2 or 4 bytes (e.g. > 4 10-bit values in 5 bytes). This flag is needed because > AV_PIX_FMT_FLAG_BITSTREAM > formats are aligned to 8 or 32 bits, and this kind of unaligned > packing needs special handling ( see below). > > Using these flags, here are some example new pixel formats: > [AV_PIX_FMT_BAYER_RGGB10] = { > .name = "bayer_rggb10", > .nb_components = 3, > .log2_chroma_w = 0, > .log2_chroma_h = 0, > .comp = { > { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 (10<<16 + 4) */ > { 0, 2, 0, 0, 655362 }, /* 5: 10/2 */ > { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 */ > }, > .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | > AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL, > }, > [AV_PIX_FMT_BAYER_RGGB12] = { > .name = "bayer_rggb12", > .nb_components = 3, > .log2_chroma_w = 0, > .log2_chroma_h = 0, > .comp = { > { 0, 2, 0, 0, 3 }, > { 0, 2, 0, 0, 6 }, > { 0, 2, 0, 0, 3 }, > }, > .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER, > }, > [AV_PIX_FMT_BAYER_GRAY10P] = { > .name = "gray10p", > .nb_components = 1, > .log2_chroma_w = 0, > .log2_chroma_h = 0, > .comp = { > { 0, 2, 0, 0, 10 }, /* Y */ > }, > .flags = AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED, > }, > [AV_PIX_FMT_BAYER_RGGB10P] = { > .name = "bayer_rggb10p", > .nb_components = 3, > .log2_chroma_w = 0, > .log2_chroma_h = 0, > .comp = { > { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 (10<<16 + 2) */ > { 0, 2, 0, 0, 655362 }, /* 5: 10/2 */ > { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 */ > }, > .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | > AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL | > AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED, > }, > [AV_PIX_FMT_BAYER_RGGB12P] = { >
[FFmpeg-devel] RFC: new packed pixel formats (machine vision)
Hi All, I want to pick up a discussion i started last week (https://ffmpeg.org/pipermail/ffmpeg-devel/2024-October/334585.html) in a new thread, with the relevant information nicely organized. This is about adding pixel formats common in machine vision to ffmpeg (though i understand some formats may also be used by cinema cameras), and supporting them as input formats in swscale so that it becomes easy to use ffmpeg for machine vision purposes (I already have such software, it will be open-sourced in good time, but right now there is a proprietary conversion layer from Basler i need to replace (e.g. by this proposal)). Example formats are 10 and 12 bit Bayer formats, where the 10 bit cannot be represented in AVPixFmtDescriptors as currently as effective bit depth for the red and blue channels is 2.5 bits, but component depths should be integers. Other example formats are 10bit gray formats where multiple values are packed without padding over multiple bytes (e.g. 4 10-bit pixels packed into 5 bytes, so not aligned to 16 or 32 bits). See https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-monochrome-pixel-formats.html for a diagram of the Mono10p and https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-raw-bayer-pixel-formats.html for diagrams of the packed and not packed bayer formats. Here a proposal for how these new formats could be encoded into AVPixFmtDescriptor, so that these can then be used in ffmpeg/swscale. I have taken care that none of the existing pixel formats or any code dealing with them would be affected, although new code would be needed to handle these new formats (av_read_image_line2, av_write_image_line2 and functions printing info about AVPixFmtDescriptors, plus swscale of course--i commit to do a full audit to ensure nothing else is missed). First, two new flags are needed (usages are shown below in the example new pixel formats). I propose: - AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL which indicates that the value in the component depths (ints) represent a 16 bit numerator and denominator packed into the int. That should be able to store any value that could ever be possible and importantly allows for the fractional bit depths needed for the bayer formats. - AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED which indicates formats that are bit-wise packed in a way that is not aligned on 1, 2 or 4 bytes (e.g. 4 10-bit values in 5 bytes). This flag is needed because AV_PIX_FMT_FLAG_BITSTREAM formats are aligned to 8 or 32 bits, and this kind of unaligned packing needs special handling ( see below). Using these flags, here are some example new pixel formats: [AV_PIX_FMT_BAYER_RGGB10] = { .name = "bayer_rggb10", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 (10<<16 + 4) */ { 0, 2, 0, 0, 655362 }, /* 5: 10/2 */ { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 */ }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL, }, [AV_PIX_FMT_BAYER_RGGB12] = { .name = "bayer_rggb12", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 3 }, { 0, 2, 0, 0, 6 }, { 0, 2, 0, 0, 3 }, }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER, }, [AV_PIX_FMT_BAYER_GRAY10P] = { .name = "gray10p", .nb_components = 1, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 10 }, /* Y */ }, .flags = AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED, }, [AV_PIX_FMT_BAYER_RGGB10P] = { .name = "bayer_rggb10p", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 (10<<16 + 2) */ { 0, 2, 0, 0, 655362 }, /* 5: 10/2 */ { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 */ }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL | AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED, }, [AV_PIX_FMT_BAYER_RGGB12P] = { .name = "bayer_rggb12p", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 3 }, { 0, 2, 0, 0, 6 }, { 0, 2, 0, 0, 3 }, }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED, }, When a AV_PIX_FMT_FLAG_BITPACKED_UNALIGNED is encountered, one needs to find out how many bytes are used to store how many samples (with a "sample" I refer to one color channel value or a gray scale value). This information can be distilled from the AVPixFmtDescriptor as follows: gray10p: sum(component_bit_depths)=10: least common multiple of 10 and 8 is 40, so there are 40/10=4 samples packed in to 40/8=5 b
Re: [FFmpeg-devel] new packed pixel formats (machine vision)
Hi Lynne, On Wed, Oct 9, 2024 at 12:52 AM Lynne via ffmpeg-devel wrote: > > On 08/10/2024 21:17, Diederick C. Niehorster wrote: > > Dear Lynne, > > > > On Tue, Oct 8, 2024 at 1:11 PM Lynne via ffmpeg-devel > > wrote: > > > > Thank you for your quick and helpful answer! However I have several > > questions. > > > >> We have support for AV_PIX_FMT_BAYER_RGGB16, since its a common Bayer > >> layout that cinema cameras output, so its definitely within the scope > >> and not some application-specific pixfmt. > >> RGGB10 is just a bitpacked version. > > > > Good! > > > >> Unfortunately, we do not directly support 10bit packed pixel formats, > >> since we can't fit them into our definition, as we only support > >> byte-aligned formats. > > > > Non byte-aligned formats can be represented with > > AV_PIX_FMT_FLAG_BITSTREAM right? I see AV_PIX_FMT_XV30BE as (the only) > > example. I am quite possibly misunderstanding. > > My first example AV_PIX_FMT_BAYER_RGGB10 is byte-aligned by the way, > > but the problem is that the R and B components would have a depth of > > 2.5 bits (10/4) in the scheme that ffmpeg uses, so can't be correctly > > defined. Though i wonder if a rounded value (one up to 3 other down to > > 2) is the solution here, since these are only informative (correct?) > > and 3+5+2=10 so would be correct for this 10bit format. > > Nope, AV_PIX_FMT_FLAG_BITSTREAM is for a very special case where all > components are aligned and repeat on a 32-bits basis. > If using it was an option, we wouldn't have bitpacked_enc or v210enc/dec. Fair enough, makes sense! > >> We treat those as codecs, for example AV_CODEC_ID_V210 and > >> AV_CODEC_ID_BITPACKED. > >> The format would have to be implemented as a codec, with a decoder that > >> accepts AV_CODEC_ID_RGGB10 and outputs AV_PIX_FMT_RGGB16, setting > >> avctx->bits_per_sample to 10 to indicate how many bits are used. > > > > Hmm, but how would that work? If i understand correctly, I would > > package the raw image data in AVPackets and use the decoder I'd write > > to turn them into AVFrames, that i can then use as i wish. > > That is a lot more complicated than adding these as pixel formats and > > having swscale support them as an input format, since then I could > > directly package the video data in an AVFrame and benefit from auto > > conversion insertion during format negotiation and feed these new > > pixel formats into anything without needing to special case with the > > extra decoder in between. > > That is how it must be. Unless you want to refactor swscale and our > entire codebase to allow such formats, which would be a lot more work. I'd like to explore this, since it is important enough that i could do the work. Of course any design should be done such that the majority of the code base (that is, all the existing usages of existing pixel formats) is unaffected. That means that the AVPixFmtDescriptor should be extended in a backwards-compatible way to accommodate the new formats. Here a first attempt, mostly to get the discussion going. First to cover all the use cases, two new flags would be needed. Their usage will be shown later below // new flag to denote that indicated bitdepths should be divided by 100, because effective bitdepth is fractional AV_PIX_FMT_FLAG_DEPTH_FRACTION100 then for RGGB10, the component bitdepths can be stored as 25, 50, 25, and the flag indictating that these values should be divided by 100 when interpreting. This allows storing many reasonable values, but is not the most flexible. An alternative would be a flag AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL And pack a 16 bit numerator and denominator into the bitdepth int. That can store any value that could ever be possible. I think this is the preferable solution. Second, a flag is needed to indicate that this is a bitpacked format (implying its not byte-aligned): AV_PIX_FMT_FLAG_BITPACKED_NONALIGNED // indicates formats that are bit-wise packed in a way that is not aligned on 1, 2 or 4 bytes (e.g. 4 10-bit values in 5 bytes) Here then some example new pixel formats: [AV_PIX_FMT_BAYER_RGGB10] = { .name = "bayer_rggb10", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 (10<<16 + 2) */ { 0, 2, 0, 0, 655362 }, /* 5: 10/2 */ { 0, 2, 0, 0, 655364 }, /* 2.5: 10/4 */ }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER | AV_PIX_FMT_FLAG_DEPTH_INT16_RATIONAL, }, [AV_PIX_FMT_BAYER_RGGB12] = { .name = "bayer_rggb12", .nb_comp
Re: [FFmpeg-devel] new packed pixel formats (machine vision)
Dear Lynne, On Tue, Oct 8, 2024 at 1:11 PM Lynne via ffmpeg-devel wrote: Thank you for your quick and helpful answer! However I have several questions. > We have support for AV_PIX_FMT_BAYER_RGGB16, since its a common Bayer > layout that cinema cameras output, so its definitely within the scope > and not some application-specific pixfmt. > RGGB10 is just a bitpacked version. Good! > Unfortunately, we do not directly support 10bit packed pixel formats, > since we can't fit them into our definition, as we only support > byte-aligned formats. Non byte-aligned formats can be represented with AV_PIX_FMT_FLAG_BITSTREAM right? I see AV_PIX_FMT_XV30BE as (the only) example. I am quite possibly misunderstanding. My first example AV_PIX_FMT_BAYER_RGGB10 is byte-aligned by the way, but the problem is that the R and B components would have a depth of 2.5 bits (10/4) in the scheme that ffmpeg uses, so can't be correctly defined. Though i wonder if a rounded value (one up to 3 other down to 2) is the solution here, since these are only informative (correct?) and 3+5+2=10 so would be correct for this 10bit format. > We treat those as codecs, for example AV_CODEC_ID_V210 and > AV_CODEC_ID_BITPACKED. > The format would have to be implemented as a codec, with a decoder that > accepts AV_CODEC_ID_RGGB10 and outputs AV_PIX_FMT_RGGB16, setting > avctx->bits_per_sample to 10 to indicate how many bits are used. Hmm, but how would that work? If i understand correctly, I would package the raw image data in AVPackets and use the decoder I'd write to turn them into AVFrames, that i can then use as i wish. That is a lot more complicated than adding these as pixel formats and having swscale support them as an input format, since then I could directly package the video data in an AVFrame and benefit from auto conversion insertion during format negotiation and feed these new pixel formats into anything without needing to special case with the extra decoder in between. > The decoder would simply call get_bits(10) and set each output pixel to > the value it gets, nothing complex. You can see how > libavcodec/bitpacked_enc.c operates and use it as a template. Thanks for these tips! ___ ffmpeg-devel mailing list ffmpeg-devel@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] new packed pixel formats (machine vision)
Hi All, I am using ffmpeg for a library to interface with machine vision cameras that i am developing (not yet released),it allows storing the streams with really nice performance directly to, e.g., x264/mp4 (thanks ffmpeg!). For this, i am looking to support some new pixel formats as input formats in swscale. A first step for this would be to describe these formats in a AVPixFmtDescriptor. Example machine vision pixel formats are: Mono10p: 10-bit luma only: 4 pixels packed into 5 bytes (i.e., no padding) BayerRG10: 10-bit color components, in bayer patterns, 1 component put into 2 bytes (10 bits data+6 bits padding) BayerRG10p: 10-bit color components, in bayer pattern, 4 color components packed into 5 bytes (really the same packing as Mono10p). And also 12 bit variants. See https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-monochrome-pixel-formats.html for a diagram of the Mono10p and https://www.1stvision.com/cameras/IDS/IDS-manuals/en/basics-raw-bayer-pixel-formats.html for diagrams of the packed and not packed bayer formats. I am wondering how to map these to AVPixFmtDescriptors. BayerRG10 is a problem: [AV_PIX_FMT_BAYER_RGGB10] = { .name = "bayer_rggb10", .nb_components = 3, .log2_chroma_w = 0, .log2_chroma_h = 0, .comp = { { 0, 2, 0, 0, ? }, { 0, 2, 0, 0, ? }, { 0, 2, 0, 0, ? }, }, .flags = AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_BAYER, }, What should be put for the component depths? for 8bit bayer, 2,4,2 is used and for 16bit 4,8,4. This I guess signifies that for rggb 1/4 components is red or blue and 2/4 are green. By this logic, the values should be 2.5,5,2.5 for a ten-bit bayer format, which is not possible (12bit is possible, with 3,6,3). How could i handle this? For Mono10p (would be something like gray10p) BayerRG10p (would be something like bayer_rggb10p) my first question is: should this be encoded as bitstreams since their pixel values are not byte-aligned? Lastly, if i figure this out, is this something that might be considered for inclusion in ffmpeg, or is there a policy/strong opinions against these machine vision formats? All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On Thu, Apr 25, 2024 at 12:50 AM Tomas Härdin wrote: > A large long term project that would help immensely with security is > moving to a proper parsing framework, rather than the present shotgun > parsing approach. But this might be such a large undertaking that it's > better to start from scratch. > > A more modest proposal is to improve subtitle support. Streaming > support could also be improved, and would be very much with the times. > The fact that we can't pass MPEG-TS through unmolested isn't great. Are point like these two, setting up a gitlab, etc, collected on a wiki somewhere? Would make writing something like the big recent funding application a bunch easier next time. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation
On Fri, Apr 19, 2024, 19:35 Zhao Zhili wrote: > > > -Original Message- > > From: ffmpeg-devel On Behalf Of > Niklas Haas > > Sent: 2024年4月19日 22:50 > > To: FFmpeg development discussions and patches > > Subject: Re: [FFmpeg-devel] [RFC] 5 year plan & Inovation > > > > On Thu, 18 Apr 2024 22:53:51 +0200 Michael Niedermayer < > mich...@niedermayer.cc> wrote: > > > A plugin system moves this patch-management to people who actually > > > care, that is the authors of the codecs and (de)muxers. > > > > A plugin system will only solve this insomuch as plugin authors will > > just host their plugin code on GitHub instead of bothering with the > > mailing list. > > > > I think it runs a good risk of basically killing the project. > > VLC is plugin based, gstreamer is plugin based too (which went t far > 😝), > I don't think plugin is that dangerous. > > Firstly, we can enable plugin interface only with enable-gpl. > > Secondly, we can have a less stable plugin interface than public API, for > our > development convenience, and encourage plugin authors to contribute to > upstream. > > > > > > Our productivity as is, is not good, many patches are ignored. > > > The people caring about these patches are their Authors and yet they > > > are powerless as they must sometimes wait many months for reviews > > > > So, rather than all of the above, what I think we should do is contract > > somebody to set up, manage, host and maintain a GitLab instance for us. > > > > This would probably be the single most cost effective boost to both > > community growth and innovation I can think of, as it will remove > > several of the major grievances and barriers to entry with the > > ML+pingspam model. > > +1. > > I can't remember how many patches I have ping. It's really frustration. > I ask for permission to commit mostly due to this. > > Now I can keep track of my own patches, but it's still not easy to filter > out > patches I'm interested to review (I can blame the email client, but blame > it > doesn't help). I'm sure I can review more patches with a new workflow. > If i recall correctly, there was a conversation not too long ago about what to do with all the SPI money. This seems to be a perfect use for it. 1. Set up and manage a gitlab instance 2. Move tickets from trac to there (possibly) 3. Move fate running to there Etc > ___ ffmpeg-devel mailing list ffmpeg-devel@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 05/29] lavu/opt: distinguish between native and foreign access for AVOption fields
On Mon, Mar 4, 2024 at 11:39 PM Marton Balint wrote: > On Mon, 4 Mar 2024, Anton Khirnov wrote: > > > Native access is from the code that declared the options, foreign access > > is from code that is using the options. Forbid foreign access to > > AVOption.offset/default_val, for which there is no good reason, and > > which should allow us more freedom in extending their semantics in a > > compatible way. > > --- > > libavutil/opt.h | 14 ++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/libavutil/opt.h b/libavutil/opt.h > > index e34b8506f8..e402f6a0a0 100644 > > --- a/libavutil/opt.h > > +++ b/libavutil/opt.h > > @@ -43,6 +43,16 @@ > > * ("objects"). An option can have a help text, a type and a range of > possible > > * values. Options may then be enumerated, read and written to. > > * > > + * There are two modes of access to members of AVOption and its child > structs. > > + * One is called 'native access', and refers to access from the code > that > > + * declares the AVOption in question. The other is 'foreign access', > and refers > > + * to access from other code. > > + * > > + * Certain struct members in this header are documented as 'native > access only' > > + * or similar - it means that only the code that declared the AVOption > in > > + * question is allowed to access the field. This allows us to extend the > > + * semantics of those fields without breaking API compatibility. > > + * > > Changing private/public status of existing fields retrospecitvely can be > considered an API break. > > > */ > > @@ -308,6 +320,8 @@ typedef struct AVOption { > > enum AVOptionType type; > > > > /** > > + * Native access only. > > + * > > * the default value for scalar options > > */ > > One could argue that it will be more difficult to get the default value of > an option (you'd have to create an object, call av_opt_set_defaults() and > finally do av_opt_get), but what I find more problematic is the > inconsistency. You are not allowed to access default_val, unless it is an > array type, in which case you might access it to get array settings, but - > oh well - not the default value. > There is no helper function for getting the default value of an option. If you disallow reading this field directly (as in one of your other patches), please add such a helper function(s), since library users need it. It should also work without instantiating the object, but directly on the class definition ___ ffmpeg-devel mailing list ffmpeg-devel@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 15/38] lavu/opt: add array options
On Sun, Mar 3, 2024 at 3:55 PM Anton Khirnov wrote: > Quoting Marton Balint (2024-02-26 20:38:46) > > The more I think about it, it is actually a broader problem. > > > > You are changing the semantics of existing AV_OPT_TYPE_xxx types. So > > previously an option with AV_OPT_TYPE_STRING used to have default value > in > > default_val.str. After your patch, it will be either default_val.str, or > > default_val.str[1], based on if it is an array or not. > > > > I think the API user safely assumed that if the option type is known to > > him, he will always find the default value in the relevant default_val > > field. It is actually a bigger issue for an array of AV_OPT_TYPE_INT, > > because previously to get the default value AVOption->default_val.i64 > was > > used, and now .str[1] should be instead... > > In my view the semantics of default_val (and offset) are only defined > when declaring options on your own object, not when accessing those > fields when declared by some other code. I also see no good reason for > any user to read these fields. > I disagree. Here an example: for a GUI using some part of ffmpeg and wanting to give the user some control over the ffmpeg operation, it would not be strange to be able to set options and either indicate which values are default, or have a "reset to defaults" button. I have written such a thing (not yet opensourced). Also the ffmpeg CLI has the ability to print the options and their defaults for a component. Can this still be done? ___ ffmpeg-devel mailing list ffmpeg-devel@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] Require compilers to support C17.
On Mon, Feb 5, 2024 at 8:59 PM Anton Khirnov wrote: > diff --git a/configure b/configure > index f72533b7d2..1bb9e23f19 100755 > --- a/configure > +++ b/configure > @@ -5517,21 +5517,20 @@ if test "$?" != 0; then > die "C compiler test failed." > fi > > -add_cppflags -D_ISOC99_SOURCE > +add_cppflags -D_ISOC11_SOURCE Not an expert, should this be D_ISOC17_SOURCE? A google shows this doesn't exist, so i guess i'm wrong. Some comment may be helpful here > add_cxxflags -D__STDC_CONSTANT_MACROS > check_cxxflags -std=c++11 || check_cxxflags -std=c++0x > > -# some compilers silently accept -std=c11, so we also need to check that the > +# some compilers silently accept -std=c17, so we also need to check that the > # version macro is defined properly > -test_cflags_cc -std=c11 ctype.h "__STDC_VERSION__ >= 201112L" && > -add_cflags -std=c11 || > -check_cflags -std=c99 > +test_cflags_cc -std=c17 ctype.h "__STDC_VERSION__ >= 201710L" && > +add_cflags -std=c17 || die "Compiler lacks C17 support" > > check_cppflags -D_FILE_OFFSET_BITS=64 > check_cppflags -D_LARGEFILE_SOURCE > > -add_host_cppflags -D_ISOC99_SOURCE > -check_host_cflags -std=c99 > +add_host_cppflags -D_ISOC11_SOURCE > +check_host_cflags -std=c17 idem ___ ffmpeg-devel mailing list ffmpeg-devel@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] avfilter: pass link YUV colorspace to ff_draw_init2
On Wed, Jan 31, 2024 at 12:17 PM Niklas Haas wrote: > > From: Niklas Haas > > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c > index fe7e6ace27..37110bc32f 100644 > --- a/libavfilter/vf_drawtext.c > +++ b/libavfilter/vf_drawtext.c > @@ -1152,7 +1152,7 @@ static int config_input(AVFilterLink *inlink) > char *expr; > int ret; > > -ff_draw_init(&s->dc, inlink->format, FF_DRAW_PROCESS_ALPHA); > +ff_draw_init(&s->dc, inlink->format, inlink->colorspace, > inlink->color_range, FF_DRAW_PROCESS_ALPHA); Should this be ff_draw_init2? > ff_draw_color(&s->dc, &s->fontcolor, s->fontcolor.rgba); > ff_draw_color(&s->dc, &s->shadowcolor, s->shadowcolor.rgba); > ff_draw_color(&s->dc, &s->bordercolor, s->bordercolor.rgba); ___ ffmpeg-devel mailing list ffmpeg-devel@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] Sovereign Tech Fund
On Mon, Jan 29, 2024 at 9:10 PM Vittorio Giovara wrote: > > On Mon, Jan 29, 2024 at 8:22 PM Michael Niedermayer > wrote: > > > > I have yet to see an actual project of "this magnitude" materialize as a > > proposal. > > > > you can suggest one ? > > > > libavscale! Not being a regular, this may not count for much. But this sounds like a great opportunity, lets not pass it up. Projects i have seen on the mailing list over the last two years or so that i remember and should be of interest: - swscale rewrite/update/extension - deal with the libavdevice situation - remove postproc - Nicolas various utility proposals for strings, options, etc - hell, even a better infrastructure for dealing with incoming patches and tests surrounding them, using pull requests, automatic CI fate runs upon incoming pull requests, etc. These are issues that are either maintenance or infrastructure work, unlikely to be funded by companies, but can help the whole project forward. I know a bunch of these are contentious, but they are worth exploring. You all probably have ten more better ideas since you know the project better. Please focus on getting something together. I fail to see serious issues. This is not a vehicle for some company interest or closed source interest to influence the project. This is not a vehicle for some unpopular minority opinion on a direction the project should take to get pushed through. This is not unfair in its distribution by nature--suggest something you'd like to work on, this is a good chance to get it funded. I understand there are potentially legitimate issues around project management that come up here again. Of course these should be discussed. But do so in parallel to moving forward and putting an application together. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] avutil/pixfmt: fix AV_PIX_FMT_RGB8 description
On Sun, Jan 14, 2024 at 4:15 PM Jeffrey Knockel wrote: > > Previously AV_PIX_FMT_RGB8 was documented as "RGB 3:3:2, > (msb)2R 3G 3B(lsb)". While the RGB 3:3:2 part is correct, the latter > part should be: (msb)3R 3G 2B(lsb). This commit also updates the > format's pixdesc description to be (msb)3R 3G 2B(lsb). > > Signed-off-by: Jeffrey Knockel > --- > libavutil/pixdesc.c | 6 +++--- > libavutil/pixfmt.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c > index 0db4167934..f6d4d01460 100644 > --- a/libavutil/pixdesc.c > +++ b/libavutil/pixdesc.c > @@ -530,9 +530,9 @@ static const AVPixFmtDescriptor > av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { > .log2_chroma_w = 0, > .log2_chroma_h = 0, > .comp = { > -{ 0, 1, 0, 6, 2 },/* R */ > -{ 0, 1, 0, 3, 3 },/* G */ > -{ 0, 1, 0, 0, 3 },/* B */ > +{ 0, 1, 0, 5, 3 },/* R */ > +{ 0, 1, 0, 2, 3 },/* G */ > +{ 0, 1, 0, 0, 2 },/* B */ > }, > .flags = AV_PIX_FMT_FLAG_RGB, > }, > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index 58f9ad28bd..9c87571f49 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -83,7 +83,7 @@ enum AVPixelFormat { > AV_PIX_FMT_BGR8, ///< packed RGB 3:3:2, 8bpp, (msb)2B 3G 3R(lsb) > AV_PIX_FMT_BGR4, ///< packed RGB 1:2:1 bitstream, 4bpp, (msb)1B 2G > 1R(lsb), a byte contains two pixels, the first pixel in the byte is the one > composed by the 4 msb bits > AV_PIX_FMT_BGR4_BYTE, ///< packed RGB 1:2:1, 8bpp, (msb)1B 2G 1R(lsb) > -AV_PIX_FMT_RGB8, ///< packed RGB 3:3:2, 8bpp, (msb)2R 3G 3B(lsb) > +AV_PIX_FMT_RGB8, ///< packed RGB 3:3:2, 8bpp, (msb)3R 3G 2B(lsb) > AV_PIX_FMT_RGB4, ///< packed RGB 1:2:1 bitstream, 4bpp, (msb)1R 2G > 1B(lsb), a byte contains two pixels, the first pixel in the byte is the one > composed by the 4 msb bits > AV_PIX_FMT_RGB4_BYTE, ///< packed RGB 1:2:1, 8bpp, (msb)1R 2G 1B(lsb) > AV_PIX_FMT_NV12, ///< planar YUV 4:2:0, 12bpp, 1 plane for Y and 1 > plane for the UV components, which are interleaved (first byte U and the > following byte V) > -- > 2.34.1 LGTM. Tested. Without this patch, the output of avutil av_read_image_line2() is all wrong, with it applied it is correct. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] TRAC Spam
On Fri, Sep 22, 2023 at 10:39 AM Michael Koch wrote: > > (?i)customer.?support > (?i)customer.?care > (?i)customer.?service > > What's the meaning of (?i) and .? > I can't find that in Python syntax description. > This is regular expression syntax, not python syntax. There are some nice online tools that show you what your regular expression will match. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] Embedded documentation?
On Mon, May 1, 2023 at 12:13 PM Nicolas George wrote: > Hi. > > Three years ago, I shared some brief thoughts about embedding the > documentation in the libraries. For example, that would allow GUI > applications to open help dialogs about specific options. > > To see what it would need, I wrote the following header. I did not work > any further, because groundwork need to be laid first. But now that it > was mentioned in another thread, I think it is a good idea to show it, > to see how people like it. > +1. I assume a lot of the AVDocNode can be automatically populated from its corresponding option? We'd not want to maintain, e.g. option names and aliases in more than one place. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] Towards YUVJ removal
On Fri, Dec 9, 2022 at 1:45 PM Niklas Haas wrote: > Oh, you are right. So that presents an alternative to 4 - rather than an > encoder flag, we can tie the auto-conversion in fftools to be tied to > the strict_std_compliance check. > > I will try implementing this approach, it may be the best compromise in > that it presents a clear upgrade path that doesn't break the common > case. > > That said, there still is an encoder that has this problem: "amv". > Currently, this only advertises YUVJ, but after YUVJ removal, it would > be treated equivalently to mjpeg when `strict_std_compliance` is enabled. > > But given that this is a less common format, I think, such a regression > would not be as big a concern. (And we can still special-case it in > fftools, if we want to) > As a user of the API, I do not think hacking fftools is the way to go. I vote option 3, filter negotiation also takes color_range (etc) info into account, and auto-inserts a scale where needed. But that is only half the solution, and actually not the important part. If i have a frame source and another sink that both have declared pixel formats, i can easily check whether conversion needs to happen at all, and if so, just set up an empty filter chain and have it auto-insert the required scale filter (super!). Now i would have to also be able to: 1. have settings in the filter context (i guess) for color_range, etc, and corresponding syntax for the args argument of avfilter_graph_create_filter() and/or corresponding options to set, so that filter input and output sides can be provided with correct info 2. query my sink for what it actually provides (a bunch more functions ala av_buffersink_get_format() are needed to get color_range, etc) I probably don't oversee the problem, so may be missing more that needs to be done, or be off the mark here. I also agree with Henrik that a way to query encoders for what they support besides formats is critical, and that this should ideally be context-senstive (so you can set certain options, and then query what remains of supported pix_fmts, color_range, etc). All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] swscale rgbaf32 input/output support
Hi, On Mon, Oct 31, 2022 at 1:33 AM wrote: > > From: Mark Reid > > This patch series adds swscale input/output support for the packed rgb float > formats. > A few of the filters also needed support the larger 96/128 bit packed pixel > sizes. > > I also plan to eventually add lossless unscaled conversions between the > planer and packed formats. > > changes since v1 > * output correct alpha is src doesn't have alpha I do not have the knowledge to review this patchset, but let me say i am very happy to see it! Thanks for the work. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/videotoolboxenc: Add CBR option to H264 and HEVC encoder
On Fri, Aug 26, 2022 at 1:42 PM Sebastian Beckmann wrote: > > Adds an option to use constant bitrate instead of average bitrate to the > videotoolbox encoders. This is enabled via -constant_bit_rate true. > macOS 13 is required for this option to work. > > Signed-off-by: Sebastian Beckmann > --- > libavcodec/videotoolboxenc.c | 37 +--- > 1 file changed, 34 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c > index 823e5ad94e..9eb6fe09a2 100644 > --- a/libavcodec/videotoolboxenc.c > +++ b/libavcodec/videotoolboxenc.c > @@ -101,6 +101,7 @@ static struct{ > CFStringRef kVTCompressionPropertyKey_RealTime; > CFStringRef kVTCompressionPropertyKey_TargetQualityForAlpha; > CFStringRef kVTCompressionPropertyKey_PrioritizeEncodingSpeedOverQuality; > +CFStringRef kVTCompressionPropertyKey_ConstantBitRate; > > CFStringRef > kVTVideoEncoderSpecification_EnableHardwareAcceleratedVideoEncoder; > CFStringRef > kVTVideoEncoderSpecification_RequireHardwareAcceleratedVideoEncoder; > @@ -164,6 +165,7 @@ static void loadVTEncSymbols(){ > "TargetQualityForAlpha"); > GET_SYM(kVTCompressionPropertyKey_PrioritizeEncodingSpeedOverQuality, > "PrioritizeEncodingSpeedOverQuality"); > +GET_SYM(kVTCompressionPropertyKey_ConstantBitRate, "ConstantBitRate"); > > > GET_SYM(kVTVideoEncoderSpecification_EnableHardwareAcceleratedVideoEncoder, > "EnableHardwareAcceleratedVideoEncoder"); > @@ -236,6 +238,7 @@ typedef struct VTEncContext { > int realtime; > int frames_before; > int frames_after; > +bool constant_bit_rate; > > int allow_sw; > int require_sw; > @@ -1073,12 +1076,22 @@ static bool vtenc_qscale_enabled(void) > return !TARGET_OS_IPHONE && TARGET_CPU_ARM64; > } > > +// constant bit rate only on Macs with Apple Silicon running macOS 13 > (Ventura) or newer > +static bool vtenc_constant_bit_rate_enabled(void) > +{ > +if (__builtin_available(macOS 13, *)) > +return !TARGET_OS_IPHONE && TARGET_CPU_ARM64; > +else > +return false; > +} vtenc_constant_bit_rate_available() may be a better name for this function, its the user that enables it through the flag you added > static int vtenc_create_encoder(AVCodecContext *avctx, > CMVideoCodecType codec_type, > CFStringRef profile_level, > CFNumberRef gamma_level, > CFDictionaryRef enc_info, > CFDictionaryRef pixel_buffer_info, > +bool constant_bit_rate, > VTCompressionSessionRef *session) > { > VTEncContext *vtctx = avctx->priv_data; > @@ -1122,6 +1135,11 @@ static int vtenc_create_encoder(AVCodecContext > *avctx, > return AVERROR_EXTERNAL; > } > > +if (constant_bit_rate && !vtenc_constant_bit_rate_enabled()) { > +av_log(avctx, AV_LOG_ERROR, "Error: -constant_bit_rate true not > available for encoder.\n"); > +return AVERROR_EXTERNAL; > +} > + > if (avctx->flags & AV_CODEC_FLAG_QSCALE) { > quality = quality >= 100 ? 1.0 : quality / 100; > quality_num = CFNumberCreate(kCFAllocatorDefault, > @@ -1139,9 +1157,16 @@ static int vtenc_create_encoder(AVCodecContext > *avctx, > &bit_rate); > if (!bit_rate_num) return AVERROR(ENOMEM); > > -status = VTSessionSetProperty(vtctx->session, > - > kVTCompressionPropertyKey_AverageBitRate, > - bit_rate_num); > +if (constant_bit_rate) { > +status = VTSessionSetProperty(vtctx->session, > + > compat_keys.kVTCompressionPropertyKey_ConstantBitRate, > + bit_rate_num); > +} else { > +status = VTSessionSetProperty(vtctx->session, > + > kVTCompressionPropertyKey_AverageBitRate, > + bit_rate_num); > +} > + > CFRelease(bit_rate_num); > } > > @@ -1530,6 +1555,7 @@ static int vtenc_configure_encoder(AVCodecContext > *avctx) > gamma_level, > enc_info, > pixel_buffer_info, > + vtctx->constant_bit_rate, > &vtctx->session); > > init_cleanup: > @@ -2532,6 +2558,7 @@ static int vtenc_populate_extradata(AVCodecContext > *avctx, > gamma_level, > enc_info, > pixel_buffer_info, > + vtctx->constant_bit_rate, >
Re: [FFmpeg-devel] Error in input
On Tue, Jul 12, 2022, 13:02 Abhishek Gorecha wrote: > Hi, When we try to give rtp stream as input we are not able to process. > Can anyone help us out here > > > > Wrong list. Post on ffmpeg-user. Also provide full details, like this no > one can help you of course ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 07/21] avdevice/avdevice: Revert "Deprecate AVDevice Capabilities API"
On Tue, May 10, 2022 at 7:25 PM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > This reverts commit 4f49ca7bbc75a9db4cdf93f27f95a668c751f160. The next > > few patches clean up the API and implement this capability for > > avdevice/dshow. > > > > Bumping avformat and avdevice version. > > > > Signed-off-by: Diederick Niehorster > > --- > > libavdevice/avdevice.c | 71 ++ > > libavdevice/avdevice.h | 5 --- > > libavdevice/version.h | 4 +-- > > libavformat/avformat.h | 21 + > > libavformat/version.h | 2 +- > > 5 files changed, 89 insertions(+), 14 deletions(-) > > > > diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c > > index 43d3406670..d367189532 100644 > > --- a/libavdevice/avdevice.c > > +++ b/libavdevice/avdevice.c > > @@ -28,11 +28,39 @@ > > #include "libavutil/ffversion.h" > > const char av_device_ffversion[] = "FFmpeg version " FFMPEG_VERSION; > > > > -#if FF_API_DEVICE_CAPABILITIES > > +#define E AV_OPT_FLAG_ENCODING_PARAM > > +#define D AV_OPT_FLAG_DECODING_PARAM > > +#define A AV_OPT_FLAG_AUDIO_PARAM > > +#define V AV_OPT_FLAG_VIDEO_PARAM > > +#define OFFSET(x) offsetof(AVDeviceCapabilitiesQuery, x) > > + > > const AVOption av_device_capabilities[] = { > > +{ "codec", "codec", OFFSET(codec), AV_OPT_TYPE_INT, > > +{.i64 = AV_CODEC_ID_NONE}, AV_CODEC_ID_NONE, INT_MAX, E|D|A|V }, > > +{ "sample_format", "sample format", OFFSET(sample_format), > > AV_OPT_TYPE_SAMPLE_FMT, > > +{.i64 = AV_SAMPLE_FMT_NONE}, AV_SAMPLE_FMT_NONE, INT_MAX, E|D|A }, > > +{ "sample_rate", "sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT, > > +{.i64 = -1}, -1, INT_MAX, E|D|A }, > > +{ "channels", "channels", OFFSET(channels), AV_OPT_TYPE_INT, > > +{.i64 = -1}, -1, INT_MAX, E|D|A }, > > +{ "channel_layout", "channel layout", OFFSET(channel_layout), > > AV_OPT_TYPE_CHANNEL_LAYOUT, > > +{.i64 = -1}, -1, INT_MAX, E|D|A }, > > +{ "pixel_format", "pixel format", OFFSET(pixel_format), > > AV_OPT_TYPE_PIXEL_FMT, > > +{.i64 = AV_PIX_FMT_NONE}, AV_PIX_FMT_NONE, INT_MAX, E|D|V }, > > +{ "window_size", "window size", OFFSET(window_width), > > AV_OPT_TYPE_IMAGE_SIZE, > > +{.str = NULL}, -1, INT_MAX, E|D|V }, > > +{ "frame_size", "frame size", OFFSET(frame_width), > > AV_OPT_TYPE_IMAGE_SIZE, > > +{.str = NULL}, -1, INT_MAX, E|D|V }, > > +{ "fps", "fps", OFFSET(fps), AV_OPT_TYPE_RATIONAL, > > +{.dbl = -1}, -1, INT_MAX, E|D|V }, > > { NULL } > > }; > > -#endif > > + > > +#undef E > > +#undef D > > +#undef A > > +#undef V > > +#undef OFFSET > > > > unsigned avdevice_version(void) > > { > > @@ -81,18 +109,49 @@ int avdevice_dev_to_app_control_message(struct > > AVFormatContext *s, enum AVDevToA > > return s->control_message_cb(s, type, data, data_size); > > } > > > > -#if FF_API_DEVICE_CAPABILITIES > > int avdevice_capabilities_create(AVDeviceCapabilitiesQuery **caps, > > AVFormatContext *s, > > AVDictionary **device_options) > > { > > -return AVERROR(ENOSYS); > > +int ret; > > +av_assert0(s && caps); > > +av_assert0(s->iformat || s->oformat); > > +if ((s->oformat && !s->oformat->create_device_capabilities) || > > +(s->iformat && !s->iformat->create_device_capabilities)) > > +return AVERROR(ENOSYS); > > +*caps = av_mallocz(sizeof(**caps)); > > +if (!(*caps)) > > +return AVERROR(ENOMEM); > > +(*caps)->device_context = s; > > +if (((ret = av_opt_set_dict(s->priv_data, device_options)) < 0)) > > +goto fail; > > +if (s->iformat) { > > +if ((ret = s->iformat->create_device_capabilities(s, *caps)) < 0) > > +goto fail; > > +} else { > > +if ((ret = s->oformat->create_device_capabilities(s, *caps)) < 0) > > +goto fail; > > +} > > +av_opt_set_defaults(*caps); > > +return 0; > > + fail: > > +av_freep(caps); > > +return ret; > > } > > > > void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, > > AVFormatContext *s) > > { > > -return; > > +if (!s || !caps || !(*caps)) > > +return; > > +av_assert0(s->iformat || s->oformat); > > +if (s->iformat) { > > +if (s->iformat->free_device_capabilities) > > +s->iformat->free_device_capabilities(s, *caps); > > +} else { > > +if (s->oformat->free_device_capabilities) > > +s->oformat->free_device_capabilities(s, *caps); > > +} > > +av_freep(caps); > > } > > -#endif > > > > int avdevice_list_devices(AVFormatContext *s, AVDeviceInfoList > > **device_list) > > { > > diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h > > index 74e9518a8e..9724e7edf5 100644 > > --- a/libavdevice/avdevice.h > > +++ b/libavdevice/avdevice.h > > @@ -347,7 +347,6 @@ int avdevice_dev_to_app_control_message(struct > > AVFormatContext *s, > >
Re: [FFmpeg-devel] [PATCH] avformat/avformat: Schedule AVOutputFormat.data_codec for removal
Hi Andreas, On Thu, May 5, 2022 at 2:47 PM Andreas Rheinhardt wrote: > > No AVOutputFormat has this set. It is not removed immediately despite > being private because of the libavdevice<->libavformat situation. > The fact that this field is private is also the reason why no FF_API_* > define has been added. Since you bring up the libavdevice<->libavformat situation, allow me to plug this patch again https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294747.html. https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294746.html has links to previous discussions of the issue. I hope you and others who chimed in before (e.g., Nicolas, Anton) have a chance to look at this potential solution. Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 00/21] avdevice (mostly dshow) enhancements
Ping for the series, especially the first commit in the series which should spark some discussion. Thanks! Dee On Wed, Mar 30, 2022 at 2:18 PM Diederick Niehorster wrote: > > This patch series implements a series of features, mostly enhancing the > dshow avdevice, but also adding new functionality to avformat. > This whole patchset enabled users of the FFmpeg API to fully > query and control a dshow device, making FFmpeg a nice backend for any > program that needs access to, e.g., a webcam. > > Different from v3 and v4, part of the patches has now been accepted, so > only remaining features are in this set. Importantly, as per discussion > on the list ( > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html, see > especially https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281586.html), > to resolve the the unholy ABI-relationship between libavdevice and > libavformat and allow easier working on the part of the avdevice API > that lives in avformat, avdevice is now locked to a specific major and minor > version of avformat. This is documented in libavdevice/avdevice.h. > > Regarding new functionality added to avformat: > Querying the capabilities of a dshow device is also possible on a > device that is already opened. I expect/guess however that it may not be > possible to achieve that for all of the avdevices, so in principle it is > important that this patchset adds the ability to create an allocated but > unopened AVFormatContext+AVInputFormat with the new function > avformat_alloc_input_context(). This is tested in the new > device_get_capabilities example. > > Diederick Niehorster (21): > avdevice: lock to minor version of avformat > avformat: add control_message function to AVInputFormat > avdevice/dshow: implement control_message interface > avdevice: add control message requesting to show config dialog > avdevice/dshow: accept show config dialog control message > avdevice/dshow: add config dialog command for crossbar and tv tuner > avdevice/avdevice: Revert "Deprecate AVDevice Capabilities API" > avdevice/avdevice: clean up avdevice_capabilities_create > avdevice: capabilities API details no longer public > avutil/opt: document AVOptionRange min_value > max_value > avdevice: Add internal helpers for querying device capabilities > avdevice: change device capabilities option type > avdevice: improve capabilities' option API > avdevice/dshow: move audio format helpers > avdevice/dshow: when closing, set context fields back to zero > avdevice/dshow: implement capabilities API > avdevice/dshow: cosmetics > avformat: add avformat_alloc_input_context() > doc/examples: adding device_get_capabilities example > Makefile/examples: cosmetics > avdevice/dshow: capabilities query also works on opened device > > configure | 2 + > doc/examples/.gitignore| 1 + > doc/examples/Makefile | 47 +- > doc/examples/Makefile.example | 1 + > doc/examples/device_get_capabilities.c | 243 ++ > doc/indevs.texi| 34 ++ > libavdevice/avdevice.c | 177 ++- > libavdevice/avdevice.h | 111 ++--- > libavdevice/dshow.c| 641 +++-- > libavdevice/dshow_capture.h| 14 + > libavdevice/dshow_crossbar.c | 91 ++-- > libavdevice/internal.h | 66 +++ > libavdevice/utils.c| 48 ++ > libavdevice/version.h | 15 +- > libavdevice/version_major.h| 2 +- > libavformat/avformat.h | 59 ++- > libavformat/demux.c| 74 ++- > libavformat/utils.c| 5 + > libavformat/version.h | 14 +- > libavutil/avutil.h | 3 + > libavutil/macros.h | 3 + > libavutil/opt.c| 2 +- > libavutil/opt.h| 5 + > 23 files changed, 1462 insertions(+), 196 deletions(-) > create mode 100644 doc/examples/device_get_capabilities.c > > -- > 2.28.0.windows.1 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avdevice/dshow: reuse unused variables.
Ping for the below. On Tue, Apr 19, 2022 at 10:13 AM Diederick Niehorster wrote: > > Fix for f125c504d8fece6420bb97767f9e72414c26312a, requested_sample_rate > and such should be used. > > Signed-off-by: Diederick Niehorster > --- > libavdevice/dshow.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > index 1e69620880..5946a72cc2 100644 > --- a/libavdevice/dshow.c > +++ b/libavdevice/dshow.c > @@ -1003,9 +1003,9 @@ dshow_cycle_formats(AVFormatContext *avctx, enum > dshowDeviceType devtype, > continue; > } > if ( > -(ctx->sample_rate && ctx->sample_rate != fx->nSamplesPerSec) > || > -(ctx->sample_size && ctx->sample_size != fx->wBitsPerSample) > || > -(ctx->channels&& ctx->channels!= fx->nChannels ) > +(requested_sample_rate && requested_sample_rate != > fx->nSamplesPerSec) || > +(requested_sample_size && requested_sample_size != > fx->wBitsPerSample) || > +(requested_channels&& requested_channels!= > fx->nChannels ) > ) { > goto next; > } > -- > 2.28.0.windows.1 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avdevice/dshow: remove unused variables
On Tue, Apr 19, 2022 at 8:10 AM Roger Pack wrote: > > LGTM. Not LGTM, see below > On Wed, Apr 13, 2022 at 10:15 AM James Almer wrote: > > > > Remnant from f125c504d8fece6420bb97767f9e72414c26312a > > > > Signed-off-by: James Almer > > --- > > libavdevice/dshow.c | 8 > > 1 file changed, 8 deletions(-) > > > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > > index 1e69620880..ac8b64366f 100644 > > --- a/libavdevice/dshow.c > > +++ b/libavdevice/dshow.c > > @@ -814,10 +814,6 @@ dshow_cycle_formats(AVFormatContext *avctx, enum > > dshowDeviceType devtype, > > / > > ctx->requested_framerate.num : 0; > > int requested_width = ctx->requested_width; > > int requested_height = ctx->requested_height; > > -// audio > > -int requested_sample_rate = ctx->sample_rate; > > -int requested_sample_size = ctx->sample_size; > > -int requested_channels= ctx->channels; > > > > if (IPin_QueryInterface(pin, &IID_IAMStreamConfig, (void **) &config) > > != S_OK) > > return; > > @@ -854,10 +850,6 @@ dshow_cycle_formats(AVFormatContext *avctx, enum > > dshowDeviceType devtype, > > requested_framerate = fmt_info->framerate; > > requested_width = fmt_info->width; > > requested_height = fmt_info->height; > > -} else { > > -requested_sample_rate = fmt_info->sample_rate; > > -requested_sample_size = fmt_info->sample_size; > > -requested_channels= fmt_info->channels; > > } > > av_free(fmt_info); // free but don't set to NULL to > > enable below check > > } These should be used further down when checking audio formats. I'll send a patch. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 5.0.1
Hi Michael, On Tue, Mar 29, 2022 at 11:08 PM Michael Niedermayer wrote: > > On Tue, Mar 29, 2022 at 01:40:26AM +0200, Diederick C. Niehorster wrote: > > Hi Michael, > > > > On Tue, Mar 29, 2022 at 1:31 AM Michael Niedermayer > > wrote: > > > > > > Hi all > > > > > > I intend to do a 5.0.1 release from the release/5.0 branch in the next > > > days > > > as its high time to make a new release with all the bugfixes > > > so if you want to backport something please do so > > > > Could you include the attached patch (not in master yet either) as it > > fixes a regression in 5.0.0? > > it should be in master first, bypassing master is a bad idea Agreed. The whole patch series was LGTMed by the dshow maintainer in https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294681.html, but i think all but the attached patch need more discussion first. Since this fixes a regression, could you (or someone on list) apply it to master and backport it? I do not have commit rights. Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] FFmpeg 5.0.1
Hi Michael, On Tue, Mar 29, 2022 at 1:31 AM Michael Niedermayer wrote: > > Hi all > > I intend to do a 5.0.1 release from the release/5.0 branch in the next days > as its high time to make a new release with all the bugfixes > so if you want to backport something please do so Could you include the attached patch (not in master yet either) as it fixes a regression in 5.0.0? Cheers and thanks, Dee v5-0001-avdevice-dshow-fix-regression.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 20/22] doc/examples: adding device_get_capabilities example
On Fri, Mar 25, 2022 at 6:26 PM Michael Niedermayer wrote: > > On Fri, Mar 25, 2022 at 03:10:39PM +0100, Diederick Niehorster wrote: > > This example also shows use of get_device_list API. > > > > Also improve capability API doc in avdevice.h: now point to this example > > instead of rough example code given in the header. > > > > Signed-off-by: Diederick Niehorster > > --- > > configure | 2 + > > doc/examples/.gitignore| 1 + > > doc/examples/Makefile | 1 + > > doc/examples/Makefile.example | 1 + > > doc/examples/device_get_capabilities.c | 243 + > > libavdevice/avdevice.h | 33 +--- > > 6 files changed, 249 insertions(+), 32 deletions(-) > > create mode 100644 doc/examples/device_get_capabilities.c > > maybe i forgot something but this seems not building It builds for me on MSVC. But searching these warnings and errors, i've learned some new things about C i didn't know because i mostly work in C++. I have locally fixed the -Wstrict-prototypes warnings by adding void as function parameter, and the Werror=missing-prototypes (hopefully) by declaring these internal functions static. Will include in next version, awaiting more comments first. Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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/6] Fix dshow device name/description
On Tue, Mar 22, 2022 at 3:10 PM Roger Pack wrote: > > On Tue, Mar 22, 2022 at 7:40 AM wrote: > > > > From: Romain Beauxis > > > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > > index 6039578ff9..4ee3f6e194 100644 > > --- a/libavdevice/dshow.c > > +++ b/libavdevice/dshow.c > > @@ -552,8 +552,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > > ICreateDevEnum *devenum, > > if (!device) > > goto fail; > > > > -device->device_name = av_strdup(friendly_name); > > -device->device_description = av_strdup(unique_name); > > +device->device_name = av_strdup(unique_name); > > +device->device_description = av_strdup(friendly_name); > > if (!device->device_name || !device->device_description) > > goto fail; > > > > LGTM. > The device enumeration API was added only recently to dshow, guess a > bug crept in. > Thanks! LGTM. Indeed, this was code introduced by my patch, committed to the ffmpeg repo only 91 days ago. Indeed, this should have been the other way around given the documentation of struct AVDeviceInfo. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] libavfilter: vf_scale: Properly take in->color_range into account
On Thu, Mar 3, 2022 at 1:07 PM Martin Storsjö wrote: > While swscale can be reconfigured with sws_setColorspaceDetails, > the in/out ranges also need to be set before calling > sws_init_context, otherwise the initialization might choose > fastpaths that don't take the ranges into account. > > Therefore, look at in->color_range too, when deciding on whether > the scaler needs to be reconfigured. > > Add a new member variable for keeping track of this, for being > able to differentiate between whether the scale filter parameter > "in_range" has been set (which should override whatever the input > frame has set) or whether it has been configured based on the > latest frame (which should trigger reconfiguring the scaler if > the input frame ranges change). > > Signed-off-by: Martin Storsjö > --- > Tested by me to resolve https://trac.ffmpeg.org/ticket/9576. Thanks Martin! All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC v2] avdevice: lock to minor version of avformat
Hi Andreas, On Mon, Jan 3, 2022 at 12:03 PM Diederick C. Niehorster wrote: > Hi Andreas, > > Thanks for the comments! > > On Mon, Jan 3, 2022 at 11:02 AM Andreas Rheinhardt > wrote: > > > > Diederick Niehorster: > > > As per discussion on the list ( > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html, see > > > especially > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281586.html), > > > to resolve the the unholy ABI-relationship between libavdevice and > > > libavformat and allow easier working on the part of the avdevice API > > > that lives in avformat, lock avdevice to a specific major and minor > > > version of avformat. > > > > > > Signed-off-by: Diederick Niehorster > > > --- > > > > 1. If this patch intends to make it illegal to use libavdevice together > > with libavformat with a different minor version than it was compiled > > against, then the most basic requirement of this is to actually properly > > document it and not add stuff that might cause linking failure if used > > in a way that runs afoul of your undocumented new requirements. > > Absolutely, documentation is required. Should that be in (amongst > local to the function in the header)? > I want to prepare a next version to get this discussion going. Where should i document that it is illegal to use libavdevice together with libavformat with a different minor version? The versioning documentation starting on line 47 in /libavutil/avutil.h? All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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/5] avdevice/dshow fixups
Hi Gyan, On Tue, Jan 4, 2022 at 1:05 PM Gyan Doshi wrote: > > On 2022-01-04 04:06 pm, Diederick C. Niehorster wrote: > > On Tue, Jan 4, 2022 at 5:10 AM Gyan Doshi wrote: > >> On 2022-01-04 05:02 am, Roger Pack wrote: > >>> These LGTM. Feel free to add yourself as a dshow maintainer if so > >>> interested, as well! :) > >> I assume these need to be pushed too. > > Yes thanks, please push. Would be good to get in before the release > > branch, fixing the regressions. > > Too late for that. But I will backport to 5.0 Super! Its still on time for the actual release, thats what matters. Thanks and cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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/5] avdevice/dshow fixups
On Tue, Jan 4, 2022 at 5:10 AM Gyan Doshi wrote: > On 2022-01-04 05:02 am, Roger Pack wrote: > > These LGTM. Feel free to add yourself as a dshow maintainer if so > > interested, as well! :) > > I assume these need to be pushed too. Yes thanks, please push. Would be good to get in before the release branch, fixing the regressions. Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC v2] avdevice: lock to minor version of avformat
Hi Andreas, Thanks for the comments! On Mon, Jan 3, 2022 at 11:02 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > As per discussion on the list ( > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html, see > > especially https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281586.html), > > to resolve the the unholy ABI-relationship between libavdevice and > > libavformat and allow easier working on the part of the avdevice API > > that lives in avformat, lock avdevice to a specific major and minor > > version of avformat. > > > > Signed-off-by: Diederick Niehorster > > --- > > 1. If this patch intends to make it illegal to use libavdevice together > with libavformat with a different minor version than it was compiled > against, then the most basic requirement of this is to actually properly > document it and not add stuff that might cause linking failure if used > in a way that runs afoul of your undocumented new requirements. Absolutely, documentation is required. Should that be in (amongst local to the function in the header)? I wanted to have the discussion first, or would docs be good to have ready to help the discussion? > 2. Is avdevice_version_same_minor() supposed to cause linking failures > by virtue of its AV_MAKE_MAJOR_MINOR_FUNC_NAME generated symbol name? Or > is the user actually supposed to run this function explicitly? > Anyway, it is not clear from the documentation that this function calls > abort on failure. The implementation is as suggested by Nicolas as suggested here https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/284492.html (And thanks for letting me know about preprocessor rescan, my v2 fixes most except ppc, see https://patchwork.ffmpeg.org/check/49132/, and now i know why :p). The idea is that it causes linker failure. I've tested this indeed, when trying to load incompatible avformat and avdevice dlls the dynamic linker throws an error. So strictly the function doesn't have to abort, nor does it have to be called at all, but it's nicely documenting. > > libavdevice/avdevice.c | 10 ++ > > libavdevice/version.h | 10 ++ > > libavformat/utils.c| 5 + > > libavformat/version.h | 11 +++ > > libavutil/macros.h | 3 +++ > > 5 files changed, 39 insertions(+) > > > > diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c > > index 8f460c7564..0c9b702eda 100644 > > --- a/libavdevice/avdevice.c > > +++ b/libavdevice/avdevice.c > > @@ -38,6 +38,16 @@ unsigned avdevice_version(void) > > return LIBAVDEVICE_VERSION_INT; > > } > > > > +unsigned avdevice_version_same_minor() > > +{ > > +// check version of loaded lavf has same major and minor version as > > +// this library was compiled against > > +if ((avformat_version_same_minor()) & ~0xFF != > > (LIBAVFORMAT_VERSION_INT & ~0xFF)) > > +abort(); > > + > > +return avdevice_version(); > > +} > > + > > const char * avdevice_configuration(void) > > { > > return FFMPEG_CONFIGURATION; > > diff --git a/libavdevice/version.h b/libavdevice/version.h > > index 41f568d6b0..83d8c67511 100644 > > --- a/libavdevice/version.h > > +++ b/libavdevice/version.h > > @@ -26,6 +26,7 @@ > > */ > > > > #include "libavutil/version.h" > > +#include "libavutil/macros.h" > > > > #define LIBAVDEVICE_VERSION_MAJOR 59 > > #define LIBAVDEVICE_VERSION_MINOR 1 > > @@ -48,4 +49,13 @@ > > */ > > #define FF_API_DEVICE_CAPABILITIES (LIBAVDEVICE_VERSION_MAJOR < 60) > > > > +/** > > + * avdevice_version_same_minor() expands to a function with > > + * the same minor and major version it was compiled against > > + * encoded in it. Enables locking to the minor version of > > + * other libraries they were compiled against. > > + */ > > +#define avdevice_version_same_minor > > AV_MAKE_MAJOR_MINOR_FUNC_NAME(device,LIBAVFORMAT_VERSION_MAJOR,LIBAVFORMAT_VERSION_MINOR) > > +unsigned avdevice_version_same_minor(); > > + > > #endif /* AVDEVICE_VERSION_H */ > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > index 332ba534d2..607a777c3f 100644 > > --- a/libavformat/utils.c > > +++ b/libavformat/utils.c > > @@ -63,6 +63,11 @@ unsigned avformat_version(void) > > return LIBAVFORMAT_VERSION_INT; > > } > > > > +unsigned avformat_version_same_minor() > > +{ > > +return avformat_version(); > > +} > > + > > const char *avformat_configuration(void) > > { > > return FFMPEG_CONFIGURATION; > > diff --git a/libavformat/version.h b/libavformat/version.h > > index 379a68cc7c..661d074b12 100644 > > --- a/libavformat/version.h > > +++ b/libavformat/version.h > > @@ -28,6 +28,7 @@ > > */ > > > > #include "libavutil/version.h" > > +#include "libavutil/macros.h" > > > > // Major bumping may affect Ticket5467, 5421, 5451(compatibility with > > Chromium) > > // Also please add any ticket numbers that you believe might be affected > > here > > @@ -63,4 +64,14 @@ > > > > > > #define FF_API_R_FRAME_RATE1 > > + > > +/** > > + * avforma
Re: [FFmpeg-devel] [RFC] avdevice: lock to minor version of avformat
FWIW, the macro #define AV_MAKE_MAJOR_MINOR_FUNC_NAME(name,major,minor) AV_GLUE(av,name)AV_GLUE(_version_,major)AV_GLUE(_,minor) doesn't compile on patchwork (https://patchwork.ffmpeg.org/check/49062/), but worked fine for me on MSVC. Is MSVC non-compliant somehow? Suggestions appreciated, I'm no star in C macros. In any case, this is a discussion starter, so let it not compiling not hold us back from that. Cheers, Dee On Sun, Dec 26, 2021 at 6:17 PM Diederick Niehorster wrote: > > As per discussion on the list ( > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html, see > especially https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281586.html), > to resolve the the unholy ABI-relationship between libavdevice and > libavformat and allow easier working on the part of the avdevice API > that lives in avformat, lock avdevice to a specific major and minor > version of avformat. > > Signed-off-by: Diederick Niehorster > --- > libavdevice/avdevice.c | 10 ++ > libavdevice/version.h | 10 ++ > libavformat/utils.c| 5 + > libavformat/version.h | 11 +++ > libavutil/macros.h | 2 ++ > 5 files changed, 38 insertions(+) > > diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c > index 8f460c7564..b608ec532f 100644 > --- a/libavdevice/avdevice.c > +++ b/libavdevice/avdevice.c > @@ -38,6 +38,16 @@ unsigned avdevice_version(void) > return LIBAVDEVICE_VERSION_INT; > } > > +unsigned avdevice_version_same_minor() > +{ > +// check version of loaded lavf has same major and minor version as > +// this library was compiled against > +if ((avformat_version_same_minor()) & ~0xFF != (LIBAVFORMAT_VERSION_INT > & ~0xFF)) > +abort(); > + > +return avformat_version(); > +} > + > const char * avdevice_configuration(void) > { > return FFMPEG_CONFIGURATION; > diff --git a/libavdevice/version.h b/libavdevice/version.h > index 41f568d6b0..4c511a7f53 100644 > --- a/libavdevice/version.h > +++ b/libavdevice/version.h > @@ -26,6 +26,7 @@ > */ > > #include "libavutil/version.h" > +#include "libavutil/macros.h" > > #define LIBAVDEVICE_VERSION_MAJOR 59 > #define LIBAVDEVICE_VERSION_MINOR 1 > @@ -48,4 +49,13 @@ > */ > #define FF_API_DEVICE_CAPABILITIES (LIBAVDEVICE_VERSION_MAJOR < 60) > > +/** > + * avdevice_version_same_minor() expands to a function with > + * the same minor and major version it was compiled against > + * encoded in it. Enables linking locking to the minor version > + * of other libraries they were compiled against. > + */ > +#define avdevice_version_same_minor > AV_MAKE_MAJOR_MINOR_FUNC_NAME(device,LIBAVFORMAT_VERSION_MAJOR,LIBAVFORMAT_VERSION_MINOR) > +unsigned avdevice_version_same_minor(); > + > #endif /* AVDEVICE_VERSION_H */ > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 332ba534d2..607a777c3f 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -63,6 +63,11 @@ unsigned avformat_version(void) > return LIBAVFORMAT_VERSION_INT; > } > > +unsigned avformat_version_same_minor() > +{ > +return avformat_version(); > +} > + > const char *avformat_configuration(void) > { > return FFMPEG_CONFIGURATION; > diff --git a/libavformat/version.h b/libavformat/version.h > index 379a68cc7c..2423800687 100644 > --- a/libavformat/version.h > +++ b/libavformat/version.h > @@ -28,6 +28,7 @@ > */ > > #include "libavutil/version.h" > +#include "libavutil/macros.h" > > // Major bumping may affect Ticket5467, 5421, 5451(compatibility with > Chromium) > // Also please add any ticket numbers that you believe might be affected here > @@ -63,4 +64,14 @@ > > > #define FF_API_R_FRAME_RATE1 > + > +/** > + * avformat_version_same_minor() expands to a function with > + * the same minor and major version it was compiled against > + * encoded in it. Enables linking locking to the minor version > + * of other libraries they were compiled against. > + */ > +#define avformat_version_same_minor > AV_MAKE_MAJOR_MINOR_FUNC_NAME(format,LIBAVFORMAT_VERSION_MAJOR,LIBAVFORMAT_VERSION_MINOR) > +unsigned avformat_version_same_minor(); > + > #endif /* AVFORMAT_VERSION_H */ > diff --git a/libavutil/macros.h b/libavutil/macros.h > index 2a7567c3ea..dda1bf6452 100644 > --- a/libavutil/macros.h > +++ b/libavutil/macros.h > @@ -73,6 +73,8 @@ > * @} > */ > > +#define AV_MAKE_MAJOR_MINOR_FUNC_NAME(name,major,minor) > AV_GLUE(av,name)AV_GLUE(_version_,major)AV_GLUE(_,minor) > + > #define AV_PRAGMA(s) _Pragma(#s) > > #define FFALIGN(x, a) (((x)+(a)-1)&~((a)-1)) > -- > 2.28.0.windows.1 > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Pixel format support fixes in swscale and drawutils
On Fri, Dec 24, 2021 at 4:09 AM rcombs wrote: > > This patchset is also available as a GitHub pull request for review > simplicity: > https://github.com/FFmpeg/FFmpeg/pull/380 > > - Reduce hardcoding of pixfmt lists, preferring deriving properties from > pixdesc > - Fix big-endian support for P[N]10/P[N]16 in swscale > - Fix several drawutils issues and add new pixfmt support > - Add more defensive checks in drawutils Does this patch address https://trac.ffmpeg.org/ticket/8454, and this patch https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210601040239.2690-1-val.zapod...@gmail.com/? Would be good to pick those up too if you could (not a blocker of course!) ___ ffmpeg-devel mailing list ffmpeg-devel@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 v6 11/12] avdevice/dshow: discover source color range/space/etc
Hi Hendrik, On Tue, Dec 21, 2021 at 2:35 PM Hendrik Leppkes wrote: > > On Tue, Dec 21, 2021 at 2:11 PM Diederick C. Niehorster > wrote: > > > > Hi Hendrik, > > > > Thanks for the review! Comments/questions below: > > > > On Tue, Dec 21, 2021 at 1:35 PM Hendrik Leppkes wrote: > > > > > > On Tue, Dec 21, 2021 at 1:15 PM Diederick Niehorster > > > wrote: > > > > > > > > Enabled discovering a DirectShow device's color range, space, primaries, > > > > transfer characteristics and chroma location, if the device exposes that > > > > information. Sets them in the stream's codecpars. > > > > > > > > Co-authored-by: Valerii Zapodovnikov > > > > Signed-off-by: Diederick Niehorster > > > > --- > > > > libavdevice/dshow.c | 255 +++- > > > > 1 file changed, 254 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > > > > index 4b1e942c7a..79be32b255 100644 > > > > --- a/libavdevice/dshow.c > > > > +++ b/libavdevice/dshow.c > > > > @@ -29,6 +29,31 @@ > > > > #include "libavcodec/raw.h" > > > > #include "objidl.h" > > > > #include "shlwapi.h" > > > > +// NB: technically, we should include dxva.h and use > > > > +// DXVA_ExtendedFormat, but that type is not defined in > > > > +// the MinGW headers. The DXVA2_ExtendedFormat and the > > > > +// contents of its fields is identical to > > > > +// DXVA_ExtendedFormat (see > > > > https://docs.microsoft.com/en-us/windows/win32/medfound/extended-color-information#color-space-in-media-types) > > > > +// and is provided by MinGW as well, so we use that > > > > +// instead. NB also that per the Microsoft docs, the > > > > +// lowest 8 bits of the structure, i.e. the SampleFormat > > > > +// field, contain AMCONTROL_xxx flags instead of sample > > > > +// format information, and should thus not be used. > > > > +// NB further that various values in the structure's > > > > +// fields (e.g. BT.2020 color space) are not provided > > > > +// for either of the DXVA structs, but are provided in > > > > +// the flags of the corresponding fields of Media Foundation. > > > > +// These may be provided by DirectShow devices (e.g. LAVFilters > > > > +// does so). So we use those values here too (the equivalence is > > > > +// indicated by Microsoft example code: > > > > https://docs.microsoft.com/en-us/windows/win32/api/dxva2api/ns-dxva2api-dxva2_videodesc) > > > > +typedef DWORD D3DFORMAT;// dxva2api.h header needs these types > > > > defined before include apparently in WinSDK (not MinGW). > > > > +typedef DWORD D3DPOOL; > > > > > > These come from d3d9types.h, through d3d9.h, which presumably you need > > > anyway due to the other D3D9 interfaces used in dxva2api.h, so I'm not > > > quite sure when these typedefs are required? > > > > I tried to keep the includes minimal (I don't need any of the d3d9 > > headers), and to be able to include dxva2api.h, these two typedefs > > were needed and nothing else. You prefer i include d3d9types.h instead > > of the typedefs? > > Definitely prefer including a header for those types then re-defining > them, yes. That can get a bit out of hand quickly otherwise. And these > older Windows headers are also well defined, so there shouldn't be any > issues. Ok, I'll replace with an include of d3d9types.h > > > > +#include "dxva2api.h" > > > > + > > > > +#ifndef AMCONTROL_COLORINFO_PRESENT > > > > +// not defined in some versions of MinGW's dvdmedia.h > > > > +# define AMCONTROL_COLORINFO_PRESENT 0x0080 // if set, indicates > > > > DXVA color info is present in the upper (24) bits of the dwControlFlags > > > > +#endif > > > > > > > > > > > > static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD > > > > biBitCount) > > > > @@ -54,6 +79,192 @@ static enum AVPixelFormat dshow_pixfmt(DWORD > > > > biCompression, WORD biBitCount) > > > > return avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), > > > > biCompression); // all others > > > > } > > > > > > > > +static enum AVColorRange dshow_color_range(DXVA
Re: [FFmpeg-devel] [PATCH v6 11/12] avdevice/dshow: discover source color range/space/etc
Hi Hendrik, Thanks for the review! Comments/questions below: On Tue, Dec 21, 2021 at 1:35 PM Hendrik Leppkes wrote: > > On Tue, Dec 21, 2021 at 1:15 PM Diederick Niehorster > wrote: > > > > Enabled discovering a DirectShow device's color range, space, primaries, > > transfer characteristics and chroma location, if the device exposes that > > information. Sets them in the stream's codecpars. > > > > Co-authored-by: Valerii Zapodovnikov > > Signed-off-by: Diederick Niehorster > > --- > > libavdevice/dshow.c | 255 +++- > > 1 file changed, 254 insertions(+), 1 deletion(-) > > > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > > index 4b1e942c7a..79be32b255 100644 > > --- a/libavdevice/dshow.c > > +++ b/libavdevice/dshow.c > > @@ -29,6 +29,31 @@ > > #include "libavcodec/raw.h" > > #include "objidl.h" > > #include "shlwapi.h" > > +// NB: technically, we should include dxva.h and use > > +// DXVA_ExtendedFormat, but that type is not defined in > > +// the MinGW headers. The DXVA2_ExtendedFormat and the > > +// contents of its fields is identical to > > +// DXVA_ExtendedFormat (see > > https://docs.microsoft.com/en-us/windows/win32/medfound/extended-color-information#color-space-in-media-types) > > +// and is provided by MinGW as well, so we use that > > +// instead. NB also that per the Microsoft docs, the > > +// lowest 8 bits of the structure, i.e. the SampleFormat > > +// field, contain AMCONTROL_xxx flags instead of sample > > +// format information, and should thus not be used. > > +// NB further that various values in the structure's > > +// fields (e.g. BT.2020 color space) are not provided > > +// for either of the DXVA structs, but are provided in > > +// the flags of the corresponding fields of Media Foundation. > > +// These may be provided by DirectShow devices (e.g. LAVFilters > > +// does so). So we use those values here too (the equivalence is > > +// indicated by Microsoft example code: > > https://docs.microsoft.com/en-us/windows/win32/api/dxva2api/ns-dxva2api-dxva2_videodesc) > > +typedef DWORD D3DFORMAT;// dxva2api.h header needs these types defined > > before include apparently in WinSDK (not MinGW). > > +typedef DWORD D3DPOOL; > > These come from d3d9types.h, through d3d9.h, which presumably you need > anyway due to the other D3D9 interfaces used in dxva2api.h, so I'm not > quite sure when these typedefs are required? I tried to keep the includes minimal (I don't need any of the d3d9 headers), and to be able to include dxva2api.h, these two typedefs were needed and nothing else. You prefer i include d3d9types.h instead of the typedefs? > > +#include "dxva2api.h" > > + > > +#ifndef AMCONTROL_COLORINFO_PRESENT > > +// not defined in some versions of MinGW's dvdmedia.h > > +# define AMCONTROL_COLORINFO_PRESENT 0x0080 // if set, indicates > > DXVA color info is present in the upper (24) bits of the dwControlFlags > > +#endif > > > > > > static enum AVPixelFormat dshow_pixfmt(DWORD biCompression, WORD > > biBitCount) > > @@ -54,6 +79,192 @@ static enum AVPixelFormat dshow_pixfmt(DWORD > > biCompression, WORD biBitCount) > > return avpriv_find_pix_fmt(avpriv_get_raw_pix_fmt_tags(), > > biCompression); // all others > > } > > > > +static enum AVColorRange dshow_color_range(DXVA2_ExtendedFormat *fmt_info) > > +{ > > +switch (fmt_info->NominalRange) > > +{ > > +case DXVA2_NominalRange_Unknown: > > +return AVCOL_RANGE_UNSPECIFIED; > > +case DXVA2_NominalRange_Normal: // equal to DXVA2_NominalRange_0_255 > > +return AVCOL_RANGE_JPEG; > > +case DXVA2_NominalRange_Wide: // equal to DXVA2_NominalRange_16_235 > > +return AVCOL_RANGE_MPEG; > > +case DXVA2_NominalRange_48_208: > > +// not an ffmpeg color range > > +return AVCOL_RANGE_UNSPECIFIED; > > + > > +// values from MediaFoundation SDK (mfobjects.h) > > +case 4: // MFNominalRange_64_127 > > +// not an ffmpeg color range > > +return AVCOL_RANGE_UNSPECIFIED; > > + > > +default: > > +return DXVA2_NominalRange_Unknown; > > The default return here should be an AVCOL_RANGE_ constant as well Absolutely, good catch! > > +} > > +} > > + > > +static enum AVColorSpace dshow_color_space(DXVA2_ExtendedFormat *fmt_info) > > +{ > > +enum AVColorSpace ret = AVCOL_SPC_UNSPECIFIED; > > + > > +switch (fmt_info->VideoTransferMatrix) > > +{ > > +case DXVA2_VideoTransferMatrix_BT709: > > +ret = AVCOL_SPC_BT709; > > +break; > > +case DXVA2_VideoTransferMatrix_BT601: > > +ret = AVCOL_SPC_BT470BG; > > +break; > > +case DXVA2_VideoTransferMatrix_SMPTE240M: > > +ret = AVCOL_SPC_SMPTE240M; > > +break; > > + > > +// values from MediaFoundation SDK (mfobjects.h) > > +case 4: // MFVideoTransferMatrix_BT2020_10 > > +case 5: // MFVideoTransferMatrix_BT2020_12 > > +if (fmt_info->VideoTransfer
Re: [FFmpeg-devel] [PATCH v5 10/13] avdevice/dshow: add media type info to get_device_list
Hi Andreas, On Mon, Dec 20, 2021 at 9:01 AM Diederick C. Niehorster wrote: > > On Mon, Dec 20, 2021 at 2:04 AM Andreas Rheinhardt > wrote: > > > > Diederick Niehorster: > > > // store to device_list output > > > if (av_reallocp_array(&(*device_list)->devices, > > > (*device_list)->nb_devices + 1, > > > @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > > > ICreateDevEnum *devenum, > > > av_freep(&device->device_name); > > > if (device->device_name) > > > av_freep(&device->device_description); > > > +if (device->media_types) > > > +av_freep(&device->media_types); > > > > You are duplicating freeing code here: You have code to free media_types > > both before it was put into device and after; you can avoid the latter > > by only attaching media_types to device when nothing else can fail. > > I could indeed avoid adding av_freep(&device->media_types) when only > attaching media_types to the device once it made it into the device > list array (so doing > (*device_list)->devices[(*device_list)->nb_devices]->media_types = > media_types). But that makes the code harder to read (asymmetric, > everything else is attached to the device before it goes into the > list) and the missing free while the other device members are freed > looks like a code smell. I think this is easier to read and less error > prone for future patches when done as currently. Nevermind, with some reordering, i see i can do as you suggest. Will do. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 09/13] avdevice: add info about media types(s) to AVDeviceInfo
Hi Andreas, On Mon, Dec 20, 2021 at 1:59 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h > > index 8370bbc7f2..6f24976dcc 100644 > > --- a/libavdevice/avdevice.h > > +++ b/libavdevice/avdevice.h > > @@ -457,6 +457,8 @@ void > > avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContex > > typedef struct AVDeviceInfo { > > char *device_name; /**< device name, format depends > > on device */ > > char *device_description;/**< human friendly name */ > > +enum AVMediaType *media_types; /**< array indicating what media > > types(s), if any, a device can provide. If null, cannot provide any */ > > +int nb_media_types; /**< length of media_types array, > > 0 if device cannot provide any media types */ > > Personally, I'd prefer it if this were unsigned given that negative > values don't make sense. But this is just a personal preference. I agree with you, but almost all nb_ in ffmpeg are int, so I guess its best to use that here too and avoid any surprises. I have applied all your comments, except the two i asked questions about, and you comment on patch 8 to allocate media_types on the stack, since it is to be returned to caller as you noticed yourself later. Thanks for the comments! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 12/13] avdevice/dshow: discover source color range/space/etc
On Mon, Dec 20, 2021 at 2:27 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > @@ -545,11 +759,40 @@ dshow_cycle_formats(AVFormatContext *avctx, enum > > dshowDeviceType devtype, > > } else { > > av_log(avctx, AV_LOG_INFO, " pixel_format=%s", > > av_get_pix_fmt_name(pix_fmt)); > > } > > -av_log(avctx, AV_LOG_INFO, " min s=%ldx%ld fps=%g max > > s=%ldx%ld fps=%g\n", > > +av_log(avctx, AV_LOG_INFO, " min s=%ldx%ld fps=%g max > > s=%ldx%ld fps=%g", > > vcaps->MinOutputSize.cx, vcaps->MinOutputSize.cy, > > 1e7 / vcaps->MaxFrameInterval, > > vcaps->MaxOutputSize.cx, vcaps->MaxOutputSize.cy, > > 1e7 / vcaps->MinFrameInterval); > > +if (extended_format_info) { > > +enum AVColorRange col_range = > > dshow_color_range(extended_format_info); > > +enum AVColorSpace col_space = > > dshow_color_space(extended_format_info); > > +enum AVColorPrimaries col_prim = > > dshow_color_primaries(extended_format_info); > > +enum AVColorTransferCharacteristic col_trc = > > dshow_color_trc(extended_format_info); > > +enum AVChromaLocation chroma_loc = > > dshow_chroma_loc(extended_format_info); > > +if (col_range != AVCOL_RANGE_UNSPECIFIED || col_space > > != AVCOL_SPC_UNSPECIFIED || col_prim != AVCOL_PRI_UNSPECIFIED || col_trc != > > AVCOL_TRC_UNSPECIFIED) { > > +const char *range = av_color_range_name(col_range); > > +const char *space = av_color_space_name(col_space); > > +const char *prim = > > av_color_primaries_name(col_prim); > > +const char *trc = av_color_transfer_name(col_trc); > > +av_log(avctx, AV_LOG_INFO, " (%s, %s/%s/%s", > > +range ? range : "unknown", > > +space ? space : "unknown", > > +prim ? prim : "unknown", > > +trc ? trc : "unknown"); > > +if (chroma_loc != AVCHROMA_LOC_UNSPECIFIED) { > > +const char *chroma = > > av_chroma_location_name(chroma_loc); > > +av_log(avctx, AV_LOG_INFO, ", %s", chroma ? > > chroma : "unknown"); > > +} > > +av_log(avctx, AV_LOG_INFO, ")"); > > +} > > +else if (chroma_loc != AVCHROMA_LOC_UNSPECIFIED) { > > +const char *chroma = > > av_chroma_location_name(chroma_loc); > > +av_log(avctx, AV_LOG_INFO, "(%s)", chroma ? chroma > > : "unknown"); > > +} > > This looks an awful lot like avcodec_string(). The logic is modeled after that indeed, but i only need a small part of what avcodec_string does, and don't have avcodeccontext here, so can't use avcodec_string. Is there a change you think i should make here? Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 10/13] avdevice/dshow: add media type info to get_device_list
On Mon, Dec 20, 2021 at 2:04 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > // store to device_list output > > if (av_reallocp_array(&(*device_list)->devices, > > (*device_list)->nb_devices + 1, > > @@ -412,6 +417,8 @@ dshow_cycle_devices(AVFormatContext *avctx, > > ICreateDevEnum *devenum, > > av_freep(&device->device_name); > > if (device->device_name) > > av_freep(&device->device_description); > > +if (device->media_types) > > +av_freep(&device->media_types); > > You are duplicating freeing code here: You have code to free media_types > both before it was put into device and after; you can avoid the latter > by only attaching media_types to device when nothing else can fail. I could indeed avoid adding av_freep(&device->media_types) when only attaching media_types to the device once it made it into the device list array (so doing (*device_list)->devices[(*device_list)->nb_devices]->media_types = media_types). But that makes the code harder to read (asymmetric, everything else is attached to the device before it goes into the list) and the missing free while the other device members are freed looks like a code smell. I think this is easier to read and less error prone for future patches when done as currently. Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] 5.0 release
Hi Michael, On Wed, Dec 15, 2021 at 3:52 PM Michael Niedermayer wrote: > > On Tue, Dec 14, 2021 at 05:42:01PM +0100, Diederick C. Niehorster wrote: > > Hi Michael, > > > > On Tue, Dec 14, 2021 at 5:19 PM Michael Niedermayer > > wrote: > > > > > > > > > The patches do not say in their commit messages that they have been > > > reviewed > > > maybe you want to repost them with that changed. > > > > Thanks for having a look! > > I have mentioned this here: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/289461.html > > > > Or do you mean that for each specific patch, if there were previous > > review comments, i should mention that it was updated in response to > > the review? For some that may be hard (e.g. i was asked to change the > > order of some commits). > > Patches which where approved already and not changed since then could > contain that information in the commit message. We tend to use some > Reviewed-by: ... > for that > that may speed up future review and commit > > I dont know dshow and dont have a proper setup to test it so i wont be > applying or reviewing these patches Ah, i see. I have made changes here in response to other review comment since i got that LGTM for the series. I'll contact the dshow maintainer again for that (I assume he would be the only one who should be committing my patchset?). Thanks, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] 5.0 release
Hi Michael, On Tue, Dec 14, 2021 at 5:19 PM Michael Niedermayer wrote: > > On Mon, Dec 13, 2021 at 10:27:33PM +0100, Diederick C. Niehorster wrote: > > Hi Michael, > > > > On Mon, Dec 13, 2021 at 4:26 PM Michael Niedermayer > > wrote: > > > > > > If you know of any major issues which need to be done before the release > > > do them > > > now. If you know of any issues which are release-blocking list them in a > > > reply > > > here please. > > > > Not major, but > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/289462.html > > fixes a crashing bug. I'd of course love for the whole series to be > > pushed in time for release (its been reviewed multiple times and > > LGTMed offlist by the dshow maintainer), but we'll see :). > > The patches do not say in their commit messages that they have been reviewed > maybe you want to repost them with that changed. Thanks for having a look! I have mentioned this here: https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/289461.html Or do you mean that for each specific patch, if there were previous review comments, i should mention that it was updated in response to the review? For some that may be hard (e.g. i was asked to change the order of some commits). Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] 5.0 release
Hi Michael, On Mon, Dec 13, 2021 at 4:26 PM Michael Niedermayer wrote: > > If you know of any major issues which need to be done before the release do > them > now. If you know of any issues which are release-blocking list them in a reply > here please. Not major, but https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/289462.html fixes a crashing bug. I'd of course love for the whole series to be pushed in time for release (its been reviewed multiple times and LGTMed offlist by the dshow maintainer), but we'll see :). Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] Compatibility of different library versions (was: avformat: Add internal flags for AV(In|Out)putFormat)
Top-posting on purpose: Several core developers stated to be in favor of locking lavd<->lavf together at the minor version, for instance since it solves a lot of the problems the intimate linking between the two libraries creates. Should this be turned into a patch somehow (i'm not sure how to do such linking)? If so, could someone send this patch or guide me through it, so that the discussion can be finished? Thanks! On Thu, Jun 17, 2021 at 12:45 AM James Almer wrote: > > On 6/16/2021 7:14 PM, Nicolas George wrote: > > James Almer (12021-06-16): > >> I'm not sure what you mean. I would not be against it, it's just that if we > >> were to merge lavf and lavd, this wouldn't even be something to consider. > > > > Have you not read the discussion? The benefits go way beyond the tiny > > lavf-lavd issues. > > > >>> and why you are > >>> against for other libraries. > >> Can you be more specific? > > > > When I say "I am for X" and you reply "I am not against Y", with Y⊂X and > > Y≠X, you are implicitly saying that you are against X∖Y. I proposed to > > restrict to matching versions on all libraries, you replied you were not > > against restricting for lavf-lavd, stating implicitly that you are > > against it for the libraries. > > Considering this discussion started about lavd and lavf, and at no point > in your email you said "all libraries" when suggesting version locking > anything (In fact, you mistakenly wrote "only versions from the same > version"), i figured it would be obvious i was commenting specifically > about lavd<->lavf. > > > > > So, I ask explicitly: are you against restricting to matching versions > > for all the libraries? If so, then why? > > Of course i am against doing so for all libraries. They are already > locked at the major soname level as required, and that's enough. > Lavd as is is locking certain lavf's public structs and making it > impossible to add new fields to them, which constricts development > considerably, so making their locking strict to at least the minor > soname level is a very good idea. But this isn't an issue at all for > other libraries. > I can add new fields to any public lavc struct, compile it and have a > lavf that was linked to an old lavc use it at runtime, and it will work > just fine. > I see no benefit to your suggestion that's worth removing such freedom > from the end user. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@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] [RFC] Suggestion for a Nicer Integration with GitHub
On Sat, Aug 14, 2021, 11:26 Nicolas George wrote: > > I agree. But it is important to remember that before seriously > considering any deep change, it would be necessary to actively poll the > silent individual-minority-work-majority. > Is there a mechanism to trigger such a vote? I hope the core developers have one soon so the project can get moving on this. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Suggestion for a Nicer Integration with GitHub
On Thu, Aug 12, 2021 at 3:45 PM wrote: > I agree with softworkz here. There are skilled programmers who like mailing > lists, and ones that don't. Same goes with unskilled prorammers as well. The > times are changing and the project needs to be ready for that. Not everyone > even likes mailing lists in the first place, so why not accomodate both? In the last few days there was also a discussion about how hard it is to filter out relevant/new text from mailing threads, sometimes even if proper spacing is used. I for one like helping my visual system by using interfaces that clearly distinguish additions and deletions, provide me with any context i want in place, integrate but visually clearly separate review comments, group those comments in threads, etc. That is, an interface like github offers. That is an interface that allows me to spend my energy on function, not form and busywork. If it is possible to set up using the suggested tool without detriment to people liking the mailing list style, i don't see what one could have against it. Should experience prove it leads to endless junk submissions it could be reconsidered/mitigations could be put in place. But we cannot estimate how likely that is, and should not be guided in the decision by such reasoning. If it works for git, i don't see why it wouldn't here. I hope more senior devs chime in, as well as someone with sufficient admin right that they could potentially set it up. For the record, despite being on windows, setting up git sendmail was just a matter of google and trying out the simplest of a few procedures i found, done in a jiffy. That said, setting it up and learning how to use it did seem like an extra hurdle, one i gladly took because i cared, but a waste of time nonetheless. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 33/34] avdevice/dshow: prevent NULL access
On Tue, Aug 3, 2021 at 4:22 PM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > list_options true would crash when both a video and an audio device were > > specified as input. Crash would occur on line 1588 (in this new rev) > > because ctx->device_unique_name[otherDevType] would be NULL > > > > Signed-off-by: Diederick Niehorster > > --- > > libavdevice/dshow.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c > > index f900e89988..a66f7b81fd 100644 > > --- a/libavdevice/dshow.c > > +++ b/libavdevice/dshow.c > > @@ -1519,9 +1519,9 @@ dshow_list_device_options(AVFormatContext *avctx, > > ICreateDevEnum *devenum, > > if ((r = dshow_cycle_devices(avctx, devenum, devtype, sourcetype, > > &device_filter, &device_unique_name, NULL)) < 0) > > return r; > > ctx->device_filter[devtype] = device_filter; > > +ctx->device_unique_name[devtype] = device_unique_name; > > if ((r = dshow_cycle_pins(avctx, devtype, sourcetype, device_filter, > > ranges ? &device_pin : NULL, ranges, query_type)) < 0) > > return r; > > -av_freep(&device_unique_name); > > return 0; > > } > > > > @@ -2043,6 +2043,7 @@ static int dshow_read_header(AVFormatContext *avctx) > > } > > } > > } > > +// don't exit yet, allow it to list crossbar options in > > dshow_open_device > > } > > ctx->is_running = 0; > > if (ctx->device_name[VideoDevice]) { > > > Is this an issue that can also happen on current git master? If so, then > it should be the first of this whole series, so that it can be > backported to the still supported release branches. If not, then you > should incorporate this into the patch that introduces the crash in > order to make sure that it never exists (we do not knowingly commit bugs). Thanks for the comment! Yes, the crash it fixes also occurs with current master, I'll make it the first patch in the series. /Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 00/34] avdevice (mostly dshow) enhancements
ping for the series On Sat, Jul 17, 2021 at 4:27 PM Diederick C. Niehorster wrote: > > On Wed, Jul 7, 2021 at 8:46 AM Diederick C. Niehorster > wrote: > > > > On Tue, Jul 6, 2021 at 11:20 AM Diederick Niehorster > > wrote: > > > > > > This patch series implements a series of features, mostly enhancing the > > > dshow avdevice, but also adding new functionality to avformat. > > > This whole patchset enabled users of the FFmpeg API to fully > > > query and control a dshow device, making FFmpeg a nice backend for any > > > program that needs access to, e.g., a webcam. > > > > Offlist, i received an LGTM from Roger Pack, the dshow maintainer. > > Ping for the series. > I realize that there are some unfinished discussions. In my > understanding, the proposal to lock avdevice and avformat together at > the minor version level (see thread kicked off by this message: > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html), which > has not seen detractors, resolves the issues that came up earlier > about how to go forward with avdevice, correct? That would clear the > way for re-introducing the (useful!) avdevice capabilities api, which > i guess is the most contentious part of this patch set. Curious to > hear of course if there is anything else blocking it! > > Let me know if there is anything i can facilitate, or alternatively, > if this requires discussion in some meeting the project will have, do > you have an idea when that meeting would be? > > Thanks and all the best, > Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 00/34] avdevice (mostly dshow) enhancements
On Wed, Jul 7, 2021 at 8:46 AM Diederick C. Niehorster wrote: > > On Tue, Jul 6, 2021 at 11:20 AM Diederick Niehorster > wrote: > > > > This patch series implements a series of features, mostly enhancing the > > dshow avdevice, but also adding new functionality to avformat. > > This whole patchset enabled users of the FFmpeg API to fully > > query and control a dshow device, making FFmpeg a nice backend for any > > program that needs access to, e.g., a webcam. > > Offlist, i received an LGTM from Roger Pack, the dshow maintainer. Ping for the series. I realize that there are some unfinished discussions. In my understanding, the proposal to lock avdevice and avformat together at the minor version level (see thread kicked off by this message: http://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281513.html), which has not seen detractors, resolves the issues that came up earlier about how to go forward with avdevice, correct? That would clear the way for re-introducing the (useful!) avdevice capabilities api, which i guess is the most contentious part of this patch set. Curious to hear of course if there is anything else blocking it! Let me know if there is anything i can facilitate, or alternatively, if this requires discussion in some meeting the project will have, do you have an idea when that meeting would be? Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 00/34] avdevice (mostly dshow) enhancements
On Tue, Jul 6, 2021 at 11:20 AM Diederick Niehorster wrote: > > This patch series implements a series of features, mostly enhancing the > dshow avdevice, but also adding new functionality to avformat. > This whole patchset enabled users of the FFmpeg API to fully > query and control a dshow device, making FFmpeg a nice backend for any > program that needs access to, e.g., a webcam. Offlist, i received an LGTM from Roger Pack, the dshow maintainer. ___ ffmpeg-devel mailing list ffmpeg-devel@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 10/33] fftools: provide media type info for devices
On Thu, Jun 17, 2021 at 3:41 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > fftools now print info about what media type(s), if any, are provided by > > sink and source avdevices. > > > > Signed-off-by: Diederick Niehorster > > --- > > fftools/cmdutils.c | 34 -- > > 1 file changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c > > index 4148285971..e7bd9f2644 100644 > > --- a/fftools/cmdutils.c > > +++ b/fftools/cmdutils.c > > @@ -2205,9 +2205,29 @@ double get_rotation(AVStream *st) > > } > > > > #if CONFIG_AVDEVICE > > +static void print_device_list(AVDeviceInfoList *device_list) > > +{ > > +// print devices > > +for (int i = 0; i < device_list->nb_devices; i++) { > > +printf("%s %s [%s]", device_list->default_device == i ? "*" : " ", > > +device_list->devices[i]->device_name, > > device_list->devices[i]->device_description); > > +if (device_list->devices[i]->nb_media_types > 0 && > > device_list->devices[i]->media_types) { > > +const char* media_type = > > av_get_media_type_string(device_list->devices[i]->media_types[0]); > > +printf(" (%s", media_type ? media_type : "unknown"); > > +for (int i = 1; i < device_list->devices[i]->nb_media_types; > > ++i) { > > You are shadowing the external counter variable here and are using a > different device for every iteration. Has this code actually been tested? Fixed both from your first mail. Yes, i have tested this, but only with zero or one media type per device since thats all i had. I could of course jury-rig the code to output that a device has two media types and then this didn't work (and it brought to the surface a bug in an earlier commit too). Thanks! Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 21/33] avdevice: capabilities API details no longer public
On Thu, Jun 17, 2021 at 4:46 AM Andreas Rheinhardt wrote: > > You are typedefing twice; this is only valid since C11; before that, > typedefs were subject to the one-definition-rule. This will break older > compilers for no benefit whatsoever, so please don't typedef here. > replaced by struct AVDeviceCapabilitiesQuery { ... }; Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/54] avformat: Add internal flags for AV(In|Out)putFormat
On Wed, Jun 16, 2021 at 12:45 PM Andreas Rheinhardt wrote: > Cost: It might force you to update more libraries, thereby increasing > download (or upload if you are the distributor) size. > Benefit: Besides fixing the horrible libavformat-libavdevice > relationship (we are currently not able to add a public field to > AV(In|Out)putFormat unless the ABI is open; we are not able to add new > private fields either if they are to be used in libavformat unless we > add checks for whether we deal with a device from an old version of > libavdevice that doesn't this option; we can not remove private fields > unless the ABI is open) it also makes sizeof of all our internal structs > usable in all libraries; because all the offsets are fixed, the avpriv > functions that are basically getters and setters can be removed > immediately. Structures crossing library boundaries were no problem any > more (we accidentally broke ABI in 88d80cb97 with this; it wouldn't be a > problem at all if we disallowed using libraries from different > revisions). We can also modify all avpriv symbols without caring about > compatibility with older users. Thanks both for writing in, seems to me like costs outweigh the benefits. Its certainly the easiest solution to an important part of the avdevice debate. Curious what others think! Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/54] avformat: Add internal flags for AV(In|Out)putFormat
On Wed, Jun 16, 2021 at 11:33 AM Andreas Rheinhardt wrote: > > Nicolas George: > > Andreas Rheinhardt (12021-06-16): > >> Yes, because one is allowed to use an old libavdevice together with a > >> new libavformat. > > > > Why do we allow that? What is the actual benefit? > > > AFAIK: Nothing. And I don't like it. It was my expectation that it would only make sense to use a set of dynamic libraries generated from the same build (and thus revision), not to mix and match. Is there a cost to no longer allowing (i.e. documenting) mixing of library versions? Is there a benefit, e.g. in being able to break (internal) ABI (not an issue i oversee)? (My interest here of course is moving forward with avdevice and implementing their APIs in dshow.) Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 17/33] avdevice/dshow: discover source color range/space/etc
On Tue, Jun 15, 2021 at 10:18 AM Michael Niedermayer wrote: > > On Mon, Jun 14, 2021 at 02:50:32PM -0300, James Almer wrote: > > On 6/14/2021 1:56 PM, Michael Niedermayer wrote: > > > On Fri, Jun 11, 2021 at 10:30:48PM +0200, Diederick Niehorster wrote: > > > > Enabled discovering a DirectShow device's color range, space, primaries, > > > > transfer characteristics and chroma location, if the device exposes that > > > > information. Sets them in the stream's codecpars. > > > > > > > > Signed-off-by: Diederick Niehorster > > > > Co-authored-by: Valerii Zapodovnikov > > > > Signed-off-by: Diederick Niehorster > > > > --- > > > > libavdevice/dshow.c | 250 +++- > > > > 1 file changed, 249 insertions(+), 1 deletion(-) > > > > > > breaks build on mingw64 > > > > > > make > > > CC libavdevice/dshow.o > > > src/libavdevice/dshow.c: In function ‘dshow_get_device_media_types’: > > > src/libavdevice/dshow.c:415:23: warning: unused variable ‘ctx’ > > > [-Wunused-variable] > > > struct dshow_ctx *ctx = avctx->priv_data; > > > ^~~ > > > src/libavdevice/dshow.c: In function ‘dshow_get_device_list’: > > > src/libavdevice/dshow.c:662:23: warning: unused variable ‘ctx’ > > > [-Wunused-variable] > > > struct dshow_ctx *ctx = avctx->priv_data; > > > ^~~ > > > src/libavdevice/dshow.c: In function ‘dshow_cycle_formats’: > > > src/libavdevice/dshow.c:731:13: error: unknown type name > > > ‘DXVA_ExtendedFormat’; did you mean ‘DXVA2_ExtendedFormat’? > > > DXVA_ExtendedFormat *extended_format_info = NULL; > > > ^~~ > > > DXVA2_ExtendedFormat > > > src/libavdevice/dshow.c:744:41: error: ‘AMCONTROL_COLORINFO_PRESENT’ > > > undeclared (first use in this function) > > > if (v->dwControlFlags & AMCONTROL_COLORINFO_PRESENT) > > > ^~~ > > > > $ grep -r AMCONTROL_COLORINFO_PRESENT /mingw64/x86_64-w64-mingw32/include/* > > /mingw64/x86_64-w64-mingw32/include/dvdmedia.h:#define > > AMCONTROL_COLORINFO_PRESENT 0x0080 > > > > Can you see if you can update your mingw-w64 toolchain, or at least the > > headers/crt? This is not the first time you get an error with new code. > > Old mingw headers releases lacked a lot of API and are not representative of > > what a sane toolchain is meant to have. > > Assuming that ubuntu didnt fail to update its mingw installation on OS update > This is a standard installation for mingw64 from 18.04 LTS packages > > I think you are missing my point here as you are asking me to upgrade. > What would i achieve by upgrading ? > Isnt that equivalent of not testing build with ubuntu 18 LTS anymore ? > > I think testing does make sense unless you belive theres something funky wrong > with my setup? > And if you want to drop support for some specific versions of platforms, > question > would be which platforms would that affect and it would require a test in > configure not failure to build in a random source file. The later can be a > pain > for novice users. Looks like i missed some DXVA_... ones, and i didn't know AMCONTROL_COLORINFO_PRESENT was also an issue. I will try to set up a mingw (instead of msvc that i am using) build environment. But i'll fix this in any case for the next version of patches (will be later, in waiting for reviews of other bits). On patchworks everything built correctly by the way, is that not using MinGW? All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 17/33] avdevice/dshow: discover source color range/space/etc
On Wed, Jun 16, 2021 at 12:23 AM James Almer wrote: > > I guess a configure option to look for the API used by this patch would > be needed, making this new code optional. No need luckily, i have a workaround for all but the AMCONTROL_COLORINFO_PRESENT symbol. I'll define that one if not already defined. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Global state into structure
On Thu, Dec 31, 2020 at 2:33 PM <(unknown sender)> wrote: > > This mail is about a project I have to make FFmpeg's API and > infrastructure more convenient. For a common introduction, see this > thread: > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/274167.html > > Global mutable state has annoyed developers for a long time. Log callbacks > and flags, malloc limits, etc., can only be set once. If an application > and > a library it depends on both use FFmpeg, the settings will conflict. > Recently, the lists of registered codecs, formats, protocols, etc., have > been made const, but that cost us the ability to control which components > were registered. > > I want to move all into a structure, maybe AVLibrary (or, more precisely, > several structures, one per library, pointing to each-other). Contexts > will > have a pointer to it, making the access transparent in most cases. > > Existing applications will continue to work by using a global instance of > AVLibrary, but new applications can allocate and their instance > explicitly. > > The benefits I want from this: > > - Bring back the ability to register only a few codecs / formats / > protocols. This is much more robust than a whitelist, because the > whitelist must be checked each time, while a component that is not > registered cannot be used at all. > > - Allow applications that want to use different memory allocation > functions to register their own. +1. As an API user, my main benefit would be to keep log for separate jobs going on in parallel separate. My application allows capturing data from one or more high-bandwidth machine vision cameras and storing them to file with ffmpeg. This often means multiple encodes are occurring in parallel, either because using multiple cameras or because a previous job didn't finish yet before the next one starts (these cameras generate so much data the encoder often can't quite keep up). I'd like each task to have its own log, not all of it mashed together into one. What is the intention here, have one context per library (so my filters have a different log_ctx than my muxer)? I think it would make most sense to allow user let them all use the same context, or not, depending on their use case (e.g. may want one specific filter to use a different log context as you need its output, or want it to use different memory allocation function for some reason)? In any case, i guess a bunch of different functions, like avformat_alloc_output_context2, avcodec_alloc_context3, avfilter_inout_alloc, avfilter_graph_alloc, would each need an extra input argument allowing user to set context to use. Further, AVLibrary or what it'll be called user have a user data pointer. I would like use the log callback from C++ classes, and need the user data to point to a specific class instance. This also means that av_log should provide the relevant context when calling the callback. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Internal error messages
On Thu, Dec 31, 2020 at 9:52 PM wrote: > > On 31/12/2020 13:36, Nicolas George wrote: > > This mail is about a project I have to make FFmpeg's API and > > infrastructure more convenient. For a common introduction, see this > thread: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/274167.html > > > > The way we currently report errors is fine for command-line tools > running in > > a terminal, but it does not suit GUI applications at all: they will get > a > > generic error, translated into a vague string like "invalid argument", > while > > a more precise error message that tells which argument is invalid and > how > > will go to the log, and disappear in a black-hole like > ~/.xsession-errors or > > even /dev/null; at best, the application will have registered a log > callback > > and display the whole log to the user. > > > > I want to add a new API to return an error all at once, something like > this: > > > > if (... failure condition...) > > return av_error_set(ctx, AVERROR_INVALIDDATA, > > "Syntax error in file ${file} line ${line}", > > "file", finename, > > "line", av_d2str(line), > > NULL); > > > > The complete error message will be stored into a pre-allocated > AVErrorStatus > > structure in the context, and can be then retrieved by the application > > using: > > > > av_error_get(ctx, buf); > > > > and displayed to the user in any convenient way (dialog box, HTTP > response, > > etc.). > > Finally having proper error support would be very nice; I like this idea a > lot. +1 from me too. Code using the libraries through their API has a hard (lets call it impossible) job showing detailed and informative error messages now. Indeed "invalid argument" or "I/O error" or so is little helpful often. So i really like this idea, it would make the library a lot more useful (also from tools user btw). One thing to add is this: Errors are often propagated back through various layers of function calls. Some next level may: 1. decide something is not an error/should be ignored. It should be able to unset the current error (if it was set, so something like av_error_clear(int error_code) which only unsets if error code matches) 2. wish to add information about the error. Often useful info is not available at the site where the error is generated, but only (several) levels up. Errors should be set where they are generated, and info can optionally be added as the stack is traversed back to the user's call. Kinda like catch and rethrow with added info/std::nested_exception in C++. something like av_error_set(ctx, optional_additional_error_code, "additional formatted error string, like in the usage example"); > > > > For compatibility, av_log(ctx, ...) will not only print to stderr, but > also > > keep the last line(s) of log in the AVErrorStatus buffer in ctx, so that > > code that still does the good old: > > > > av_log(ctx, AV_LOG_ERROR, "Syntax error in file %s line %d\n", > >filename, line); > > return AVERROR_INVALIDDATA; > > > > will now work ok with av_error_get(). > > What I'm a little iffy on is this last part, mostly because a lot of code > in > FFmpeg just returns an error without any av_log call, so the current error > buffer in the context will at best be unrelated, or at worst, really > misleading to the user. Agreed, this can't work consistently, I'd skip it. And it highlights also that there may be stale errors returned through av_error_get(). So: 1. av_error_get() should clear error state (by default?) 2. av_error_get should perhaps have an error_code input argument, so that an error is only returned if the error code (uppermost if nested according to my above suggestion) matches--else it is assumed we're dealing with a stale error. if error_code==0, such filtering is bypassed. By the way, av_error_get should perhaps have last in the name, like lasterr. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] Type descriptors
On Thu, Dec 31, 2020 at 2:35 PM wrote: > > This mail is about a project I have to make FFmpeg's API and > infrastructure more convenient. For a common introduction, see this > thread: > https://ffmpeg.org/pipermail/ffmpeg-devel/2020-December/274167.html > > For each simple type, including enumerations like AVColorRange and flat > structures like AVReplayGain, have a set of standardized functions for > common operations, including probably: > > - printing; > - serializing to string; > - parsing from string; > - serializing to/from compact binary form; > - freeing; > - referencing or cloning. > > These functions will have a standardized name and prototype. They will be > grouped in structures that describe a type entirely. Strong +1. Such code is spread all over the code base, and sometimes hard to discover due to variety in naming and location (as an API user, I often end up grepping ffmpeg source for a to_string or from_string function until i find one, and then grep again for example use). Consistency like the proposed struct of functions would be great! How to get the relevant info for a given type? Some type of type registry? This relates importantly to options and how to parse/print those. There is a limited set of option types (e.g. there is AV_OPT_TYPE_PIXEL_FMT but not AV_OPT_TYPE_CODEC_ID), none of which should be needed by simply linking each option to the right type description struct. Would save me half a headache when using the device capabilities API (see https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281379.html for at least two non-ideal solutions to formatting some of the information it returns--we shouldn't need any new API for this at all). Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avdevice/dshow: implement capabilities API
Reorganized a bit for easier replying. Also, while i think this is an important discussion, i do not see why it should stop de-deprecation of a good API. Deprecating the device capabilities API cleaned up avformat a bit, but various other function pointers are left. A redesign would clean them all up at once, if thats the direction taken. As such, having the device capabilities api does not create a hindrance to a redesign. As i argued, having a use example of important functionality helps, not hinders the redesign effort. And lets not forget it offers users of the current API (me at least) functionality they need now, not at some indeterminate timepoint in the future. So, lets not stop work on the current avdevice component (my patch set) while figuring out the way forward. Lets keep the below discussion separate, they are two separate points to decide on I think. What is your view on this Anton / how should this issue be decided? On Fri, Jun 11, 2021 at 5:24 PM Anton Khirnov wrote: > > It is also about new functionality, since one of the main stated > advantages of libavdevice is that it can be used transparently by all > programs that use libavformat. New libavdevice-specific APIs > go against this. > > I see a contradiction here. On one hand you're saying that the > usefulness of lavd comes from it having the same API as lavf. But then > you want to add a whole bunch of libavdevice-specific APIs. So any > program that wants to use them has to be specifically programmed for > libavdevice anyway. And libavformat API is saddled with extra frameworks > that are of no use to "normal" (de)muxing. > > At that point, why insist on accessing lavd through the lavf API? You > can have a lavd-specific API that can cleanly export everything that is > specific to capture devices, without ugly hacks that are there > currently. I do not think there is a contradiction. Note also that i did not say i "want to add a whole bunch of libavdevice-specific APIs." For (not unimporant) convenience, as an API user, i want to use some of the already existing API to enhance my use of avdevices. Quoting a bit from Nicolas' reply allows me to make the point that there is no contradiction: On Fri, Jun 11, 2021 at 3:17 PM Nicolas George wrote: > Input devices are demuxers with a few extra methods; output devices are > muxers with a few extra methods. We already have the beginning of a > class/interface hierarchy: > > formats > | > + muxers > | | > | + output devices > | > + demuxers > | > + input devices Code in my (C++) program looks just like that. I have a significantly-size class with code for accessing ffmpeg demuxers, originally written to be able to read files. Then i realized access to webcams would be important too. It was very simply added by deriving my webcam class from the general ffmpeg input class and adding a little bit of convenience functions to deal with device discovery, proper URL formatting (so users don't have to prepend "video=" for dshow). I.e., almost all of the code is shared between my class for general avformats and the avdevice class. But, for more advanced use than just streaming in packets/frames, I need to gain a bit of control over the avdevice. These are convenience things, even if not unimportant, and appropriate APIs are already available. Thats what i implemented for dshow. Right now i needed another DirectShow library to do device discovery and given a device, format discovery. The ffmpeg API is (was) already there, just needed to be implemented. I also needed a way to pause/resume (just stopping reading packets like you can do when the input is a file does not work for a realtime source, where it leads to a full packet list buffer, and a kinda nasty restart as first you get a bunch of irrelevant old frames, and may lose a few of the first ones you do want as the buffer was still full). These bits of additional API thus helps make using avdevices better, but even without them, i (and i assume many other happy users out there) already have a very workable solution. > > (I do see Mark's list of > > opportunities that a new API offer, copied below). I see Nicolas argue > > this entanglement of internals is not a problem in practice, and i > > suppose there is a certain amount of taste involved here. > > Do note that Nicolas' position in library separation is quite unorthodox > --- I am not aware of anyone else supporting it, several people strongly > disagree with it. It also disagrees with our current practice. > [...] > > The function pointers, various private APIs, contents of > AVFormatInternal, etc. This is a pretty big deal, since it restricts > what we can do to libavformat internals without breaking ABI. This may be above my pay grade, but: ABI here would be ABI between the various dlls, not user-facing right? (They are internals after all, and hidden
Re: [FFmpeg-devel] [PATCH 0/4] avdevice/dshow: implement capabilities API
Nicolas, I agree with what you said, and you obviously have given this more thought than me. Your and Anton's replies provide two by now pretty clear different views, I hope more will chime in. I will only highlight some things below. On Fri, Jun 11, 2021 at 3:17 PM Nicolas George wrote: > Input devices are demuxers with a few extra methods; output devices are > muxers with a few extra methods. We already have the beginning of a > class/interface hierarchy: > > formats > | > + muxers > | | > | + output devices > | > + demuxers > | > + input devices Exactly, this is what the class hierarchy in my program using ffmpeg also looks like. > > I agree. Thought have been given to designing this API, the efforts have > dried up before implementing the functional parts, but the design is > sound, and a good starting point to work again. Yes, so lets use it to solve a real (my) problem now. Doing so does not hamper a large redesign/reorganization effort later. > I think the problem emphasized here is not really about fields, more > about the working of the API: files are read on demand, while operate > continuously, that makes a big difference. > > But really, we already have this difference with network streams, > especially those that do not have flow control, for example those in > multicast. These network streams have aspects of protocols, but also > aspect of devices. > > And the answer is NOT to separate libavio from libavformat: protocols > and formats mesh with each other, see the example of RTP. :) I have been wondering why protocols are in formats, and not in a separate libavprotocol or libavtransport or so. But i understand indeed that some of these, like devices, must be intertwined. > In practice, that would look like this: > > application > → libavformat API > → libavdevice compatibility wrapper >→ libavdevice API > → wrapper for old-style device > → actual device > > While the useful code is just: > > application > → libavformat/device API > → actual device > > That's just an insane idea, a waste of time. Hmm, fair enough! > > > Anyway, out of Mark's options i'd vote for a separate new AVDevice > > API, and an adapter component to expose/plug in AVDevices as formats. > > I do not agree. I think we need to think this globally: shape our > existing APIs into a coherent object-oriented hierarchy of > classes/interfaces. This is not limited to formats and devices, we > should include protocols in the discussion, and probably codecs and > filters too. This is an interesting point. It would force a discussion about which "classes" should sensibly have access to internals of other classes (i.e. alike public/protected/private), and thereby make completely explicit how each component is linked to the others. It seems to me that physical organization both into a folder structure and into different dlls is rather secondary. As you wrote elsewhere, a lot of the libraries depend on each other, and that makes sense. > And to handle the fact that devices and network streams are > asynchronous, the main API needs to be asynchronous itself. > > Which brings me to my project to redesign libavformat around an event > loop with callbacks. > > I have moderately started working on it, by writing the documentation > for the low-level single-thread event loop API. Then I need to write the > documentation for the high-level multi-thread scheduler API. Then I can > get to coding. Looking forward to it, sounds useful! Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 23/33] avutil/opt: add av_opt_print_num
On Fri, Jun 11, 2021 at 10:38 PM Diederick Niehorster wrote: > > This function allows formatting an option value stored in a double (such > as the min and max fields of an AVOption, or min_value and max_value of > an AVOptionRange) properly, e.g. 1 for a AV_OPT_TYPE_PIXEL_FMT -> yuyv422. > > Useful when printing more info about an option than just its value. > Usage will be shown in upcoming device_get_capabilities example. This solution is not perfect, since multiple of the options returned are not formatted properly now: - "codec", which is just printed as int since there is no AV_OPT_TYPE_CODEC or something like that - "frame_size"/"window_size": these are reported as three components: "pixel_count", "width", "height", none of which have a known type (and thus we default to printing the provided double. The other solution which enables handling this is to add a new function avdevice_capabilities_print, which you pass a single AVOptionRange* (not AVOptionRanges** as that would create many lines of output) and it returns a formatted string. In the implementation of that function, since we know all the options (and related components), we can special-case the formatting where needed so that it all comes out right. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 25/35] avutil/opt: add av_opt_to_string
On Thu, Jun 10, 2021 at 12:08 PM Diederick C. Niehorster wrote: > > On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster > wrote: > > So in av_opt_get(), I'd do something like this: > > AVBPrint buf; > > av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); > > // 1. call internal print function, with &buf > > // ... > > // 2. provide output to caller > > if (!av_bprint_is_complete(&buf)) > > { > > av_bprint_finalize(&buf,NULL); > > return AVERROR(ENOMEM); > > } > > ret = av_bprint_finalize(&buf,out_val); > > return ret<0 ? ret : 0; > > > > I'll experiment with that. > > Ok, i have experimented with this, and hit some problems quickly. > There are at least the following issues: > 1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a > move over to AVBPrint? leaving AVBPrint empty is not the same as a > null, and does not signal null. > 2. some cases forward to other functions, most importantly > AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play > with AVBPrint, but just plain char** or similar. Should i use a local > buffer in that case and then copy/print its contents into the > AVBPrint? That seems to not improve the situation. > This is assuming we'd want to reuse the guts of av_opt_get for this > new function, which i think we would. Not doing so would mean a lot of > code duplication. > > And there is also that this would be the only av_opt_* function > returning output through a AVBPrint instead of plain arrays. Is that > inconsistency in the API worth usually avoiding a dynamic allocation > in code that'll not be used in a critical path? I've just sent v2 of the patch set. Given what i have written above, i have not moved the function to using AVBPrint, so i can reuse av_opt_get internals and don't introduce inconsistency in the options API. That said, moving it _all_ over to your better strings API proposal[1] sounds like a pretty sweet thing to do! I also hear you on your idea for better options. While the function i add here allows me to format pixel_format and sample_formats correctly because they have their own options type, I still have the issue that another option "codec" is just printed as int, as there is no AV_OPT_TYPE_CODEC, or otherwise information that it should be turned into a codec name... Hope that something like your proposal will make that a thing of the past. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 24/35] avutil/opt: AVOptionRange gains is_set field.
On Tue, Jun 8, 2021 at 1:45 AM Andreas Rheinhardt wrote: > This has absolutely nothing to do with full/limited range, but rather > whether the AVOptionRange contains an interval or a single value. > (Not that I know why this needs to be set explicitly and not implicitly > via is_range = value_min < value_max.) Hmm, instead of adding a new field, would a bit of documentation be a good idea? There are three cases: 1. output is a range. Then value_min < value_max. 2. output is set but not a range (single value): Then value_min==value_max. (We probably shouldn't remove the is_range flag? OTOH, it is not used in the ffmpeg code base at all...) 3. output is not set: then value_min>value_max (and perhaps special values value_min=0, value_max=-1. A pair of special values does not have the problem that a single special value has: how to distinguish it from an actual value) Then users can easily determine themselves: is_set = value_min > value_max (or preferrably value_min==0 && value_max==-1) is_range = value_min < value_max So two questions: 1. What do you think of documenting case 3 instead of adding a new field to API? 2. What do you think of removing is_range? Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avdevice/dshow: implement capabilities API
Let me respond on two levels. Before exploring the design space of a separation of libavdevice and libavformat below, I think it is important to first comment on the current state (and whether the AVDevice Capabilities part of my patch series should be blocked by this discussion). Importantly, I would suppose that any reorganization of libavdevice and libavformat and redesign of the libavdevice API must aim to offer at least the same functionality as the current API, that is, an avdevice should be able to be queried for what devices it offers (get_device_list), should for each device provide information about what formats it accepts/can provide (create_device_capabilities/free_device_capabilities) and should be able to be controlled through the API (control_message). Perhaps these take different forms, but same functionality should be offered. As such, having AVDevice Capabilities API implemented for one of the devices should help, not hamper, redesign efforts because it shows how this API would actually be used in practice. Fundamental changes such as a new avdevice API will be backwards incompatible no matter what, so having one more bit of important functionality (create_device_capabilities/free_device_capabilities) implemented doesn't create a larger threshold to initiating such a redesign effort. Instead, it forces that all the current API functionality is thought out as well during the redesign effort and nothing is forgotten. I thus argue that its a good thing to bring back the AVDevice Capabilities API, since it helps, not hinders the redesign effort. And lets not forget it offers users of the current API functionality (me at least) they need now, not at some indeterminate timepoint in the future. On Wed, Jun 9, 2021 at 10:33 PM Anton Khirnov wrote: > Look through the threads > [...] Thanks for the pointers! > The problem is that libavdevice is a separate library from libavformat, > but fundamentally depends on accessing libavformat internals. Ah ok, so this is at first instance about cleanup/separation, not necessarily about adding new functionality (I do see Mark's list of opportunities that a new API offer, copied below). I see Nicolas argue this entanglement of internals is not a problem in practice, and i suppose there is a certain amount of taste involved here. Nothing wrong with that. I guess for me personally that it is a little funky to have to add/change things in AVFormat when changing the AVDevice API, and that it may be good to for the longer term look at disentangling them. I will get back to that below, in response to some quotes of Mark's messages last January. Mark's (non-exhaustive) list of opportunities a libavdevice API redesign offers (numbered by me): On 20/01/2021 12:41, Mark Thompson wrote: > 1. Handle frames as well as packets. >1a. Including hardware frames - DRM objects from KMS/V4L2, D3D surfaces from Windows desktop duplication (which doesn't currently exist but should). > 2. Clear core option set - currently almost everything is set by inconsistent private options; things like pixel/sample format, frame/sample rate, geometry and hardware device should be common options to all. > 3. Asynchronicity - a big annoyance in current recording scenarios with the ffmpeg utility is that both audio and video capture block, and do so on the same thread which results in skipped frames. > 4. Capability probing - the existing method of options which log the capabilities are not very useful for API users. 1 and 3 i cannot speak to, but 4 is indeed what i ran into: the current state of most avdevices is not useful at all for an API user like me when it comes to capability probing (not a reason though to get rid of the whole API, but to wonder why it wasn't implemented. while nobody apparently bothered to do it before me, i think there will be more than just me who will actually use it). Currently I'd have to issue device specific options on a not-yet opened device, listen to the log output, parse it, etc. But the current API already solves this, if only it was implemented. A clear core option set would be nice indeed. And the AVDevice Capabilities API actually offers a start at that, since it lists a bunch of options that should be relevant to query (and set) for each device in the form of ff_device_capabilities (in my patchset), or av_device_capabilities before Andreas' patch removing it in January. I don't think its complete, but its a good starting point. Mark Thompson (2021-01-25): > * Many of those are using it via the ffmpeg utility, but not all. Indeed, i am an (aspiring) API user, of the dshow device specifically, and possibly v4l2 later (but my project is Windows-only right now). Currently hampered by lack of some API not being implemented for dshow, hence my patch set. > * The libavdevice API is the libavformat API because it was originally > split out from libavformat, and it has the nice property that devices > and files end up being interchangable in so
Re: [FFmpeg-devel] [PATCH 25/35] avutil/opt: add av_opt_to_string
On Thu, Jun 10, 2021 at 11:39 AM Diederick C. Niehorster wrote: > So in av_opt_get(), I'd do something like this: > AVBPrint buf; > av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); > // 1. call internal print function, with &buf > // ... > // 2. provide output to caller > if (!av_bprint_is_complete(&buf)) > { > av_bprint_finalize(&buf,NULL); > return AVERROR(ENOMEM); > } > ret = av_bprint_finalize(&buf,out_val); > return ret<0 ? ret : 0; > > I'll experiment with that. Ok, i have experimented with this, and hit some problems quickly. There are at least the following issues: 1. how to deal with AV_OPT_ALLOW_NULL in av_opt_get, if its guts use a move over to AVBPrint? leaving AVBPrint empty is not the same as a null, and does not signal null. 2. some cases forward to other functions, most importantly AV_OPT_TYPE_DICT, which uses av_dict_get_string. These do not play with AVBPrint, but just plain char** or similar. Should i use a local buffer in that case and then copy/print its contents into the AVBPrint? That seems to not improve the situation. This is assuming we'd want to reuse the guts of av_opt_get for this new function, which i think we would. Not doing so would mean a lot of code duplication. And there is also that this would be the only av_opt_* function returning output through a AVBPrint instead of plain arrays. Is that inconsistency in the API worth usually avoiding a dynamic allocation in code that'll not be used in a critical path? Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 25/35] avutil/opt: add av_opt_to_string
(NB: I have reorganized your reply a bit to make it for me to respond to) On Wed, Jun 9, 2021 at 1:54 PM Nicolas George wrote: > > The critical part is the API of the new public function, because this we > cannot fix later. > > Unfortunately, to get a good API, you will not be able to reuse the > implementation of av_opt_get() as it is. Therefore, you will probably > need two changes: (1) change the implementation of av_opt_get() and (2) > add the new function. > > Whether this should be done in one or two patches depends on the > specifics, on the amount of code in each change and how much they cover > each other. > > In the meantime, as I suggested, I think using AVBPrint is the best > option: > > void av_num_opt_bprint(AVBPrint *bp, AVOptionType type, double val); Ok, see if i understand this correctly. To use AVBPrint in this new API (which when reading up on it, seems like a good idea since it does something similar to C++'s std::string's small string optimization, and many string here will be very short, so no dynamic allocations), i would need to change the guts of av_opt_get() that i would like to reuse to take an AVBPrint. That would be pretty similar to the current uint8_t buf[128] currently used in the function, but remove the arbitrary (if rarely hit) size limit. What is the size of the internal buffer of AVBPrint? So in av_opt_get(), I'd do something like this: AVBPrint buf; av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); // 1. call internal print function, with &buf // ... // 2. provide output to caller if (!av_bprint_is_complete(&buf)) { av_bprint_finalize(&buf,NULL); return AVERROR(ENOMEM); } ret = av_bprint_finalize(&buf,out_val); return ret<0 ? ret : 0; I'll experiment with that. > > I have long-term projects of rewriting the options system, with each > type (channel layouts, pixel formats, color ranges, etc.) coming with a > descriptor for printing and parsing it, and a de-centralized way of > adding new types. Unfortunately, first I need to convince people here > that we need a better strings API That sounds nice. Lets keep things here as they are then. Have you laid out your plans somewhere i can read about them? Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avdevice/dshow: implement capabilities API
On Wed, Jun 9, 2021 at 8:18 PM Anton Khirnov wrote: > > Quoting Diederick C. Niehorster (2021-06-05 16:51:32) > > On Sat, Jun 5, 2021 at 4:36 PM Anton Khirnov wrote: > > > Sorry to rain on your parade, but I don't think we should go ahead with > > > this before deciding what is to be done with libavdevice. The last > > > discussion about it died without being resolved, but the issues are > > > still present. > > > > I understand. I realize I'm new here: Is there a timeline for such a > > discussion and could the discussion benefit from a real usage example > > such as this patch series? I guess a big change in libavdevice should > > at least offer the functionality the current implementation offers (or > > theoretically offers :p). I really like the current design where an > > avdevice can just be used through the avformat interface. Add > > avdevice_register_all(); and Bob's your uncle! > > There is no timeline, it depends on someone sitting down and doing the > work. The options proposed so far were > 1) merging libavdevice into libavformat > 2) making libavdevice into an independent library with an independent >API > 3) moving libavdevice functionality into ffmpeg.c Thanks for providing the explored options. What problem is there in the way things currently are that these would be solving? > I volunteered to do 1), but was stopped by some issues and Mark > volunteering to do 2). But Mark did not progress far on that apparently, > so now we are stuck. Maybe we should do a technical committee vote on > it. While not being familiar with the alternative, as said, from my perspective whats great about the current setup is that avdevices act just like formats, making them easy to integrate. And for devices that actually implement functions like get_device_list, control_message and create_device_capabilities, they are also directly explorable and controllable like an independent API would presumably allow. Best of both worlds in my book. Could you point me to previous discussions regarding options 1 and 2, if they are available somewhere to read, so i can have a more informed opinion (in case that would carry any weight)? Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 13/35] avdevice: adding control message requesting to show config dialog
On Wed, Jun 9, 2021 at 1:20 PM Nicolas George wrote: > Thanks for explaining. Then "data: int (device-specific)" should be > enough, if you also document the device-specific value in the user > documentation of each device. > > That means in patch 14, "Documentation of dialog variable:" should be in > doc/devices.text, I think. That documentation seems to be (to me) for users of the command line tools, who wouldn't be able to use this command message interface. That said, its the best/only place in the documentation where the int could be documented. Would something like an extra section after https://ffmpeg.org/ffmpeg-devices.html#Examples-2, titled for example "Libavdevice user notes" be what you have in mind? Thanks, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 10/35] fftools: provide media type info for devices
On Wed, Jun 9, 2021 at 1:15 PM Nicolas George wrote: > > What matters is not what you see in the console but what data is really > written on the stream. Programs that read from the ffmpeg process and > use the output to build a command line, all in a binary-clean way, > should succeed. > > You can test with Perl (you can probably achieve the same with any > language, but I know Perl, and I know it will not automagically mess > things up): > > my $out = `ffmpeg -devices`; > my $dev = $out =~ /(Microphone.*)/; > print "Using \"$dev\"\n"; > system "ffmpeg", "-f", "dshow", "-i", $dev; > > If the “Using "..."” message is mangled but the command line succeeds, > then the issue is not ffmpeg's problem. I don't know perl well, but after modifying the program a bit i got it to capture the device name and call ffmpeg with it: my $out = `ffmpeg -sources dshow`; ($dev) = ( $out =~ /(Microphone.*) \[/); print "Using \"$dev\"\n"; system "ffmpeg", "-f", "dshow", "-i", "audio=\"$dev\""; The regex looks the way it does because the full output line is: "Microphone Array (Intel┬« Smart Sound Technology (Intel┬« SST)) [@device_cm_{33D9A762-90C8-11D0-BD43-00A0C911CE86}\wave_{A6F097A2-0BFA-4F4C-BF22-D4531781DA10}] (audio)" Running that perl script, i get the following output (skipping ffmpeg header): Using "Microphone Array (Intel┬« Smart Sound Technology (Intel┬« SST))" [dshow @ 01B93364A7C0] Could not find audio only device with name [Microphone Array (Intel® Smart Sound Technology (Intel® SST))] among source devices of type audio. [dshow @ 01B93364A7C0] Searching for audio device within video devices for Microphone Array (Intel® Smart Sound Technology (Intel® SST)) [dshow @ 01B93364A7C0] Could not find audio only device with name [Microphone Array (Intel® Smart Sound Technology (Intel® SST))] among source devices of type video. audio=Microphone Array (Intel® Smart Sound Technology (Intel® SST)): I/O error So something isn't going quite right there. To be sure, running ffmpeg -f dshow -i audio="Microphone Array (Intel® Smart Sound Technology (Intel® SST))" directly from cmd does work. Did i make a perl error, or is there an ffmpeg problem? Thanks, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 17/35] avdevice/dshow: discover source color range/space/etc
On Tue, Jun 8, 2021 at 5:01 PM Michael Niedermayer wrote: > On Tue, Jun 08, 2021 at 01:03:56AM +0200, Diederick Niehorster wrote: > > Enabled discovering a DirectShow device's color range, space, primaries, > transfer characteristics and chroma location, if the device exposes that > information. Sets them in the stream's codecpars. > > > > Signed-off-by: Diederick Niehorster > > Co-authored-by: Valerii Zapodovnikov > > --- > > libavdevice/dshow.c | 231 +++- > > 1 file changed, 230 insertions(+), 1 deletion(-) > > seems to break build on mingw64 here > > CC libavdevice/dshow.o > src/libavdevice/dshow.c:58:44: error: unknown type name > ‘DXVA_ExtendedFormat’ > static enum AVColorRange dshow_color_range(DXVA_ExtendedFormat* fmt_info) > ^~~ > Oh no! Seems all those are not defined in the MinGW headers. Luckily there is the equivalent (see https://docs.microsoft.com/en-us/windows/win32/medfound/extended-color-information#color-space-in-media-types) DXVA2_ExtendedFormat struct which is defined in the MinGW headers. So i have switched to using that (well documented whats going on), seems to work fine. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 25/35] avutil/opt: add av_opt_to_string
On Tue, Jun 8, 2021 at 4:32 PM Nicolas George wrote: > > Diederick Niehorster (12021-06-08): > Please wrap this. Will do. > > -int av_opt_get(void *obj, const char *name, int search_flags, uint8_t > > **out_val) > > +static int print_option(void* dst, enum AVOptionType type, int > > search_flags, uint8_t** out_val) > > I really do not like the fact that it always makes a dynamic allocation, > especially since in most cases we know the buffer has a small bounded > size. > > I'd rather have AVWriter here, but in the meantime, please consider > using AVBPrint. The guts of this function is unchanged from av_opt_get, just moved here, should i address this in this patch, or is it a separate issue? Happy to do either. > > { > > -void *dst, *target_obj; > > -const AVOption *o = av_opt_find2(obj, name, NULL, 0, search_flags, > > &target_obj); > > -uint8_t *bin, buf[128]; > > > +uint8_t* bin, buf[128]; > > Here and everywhere, the pointer mark belongs with the variable name. > This is an obvious example of why: written like this, it seems that bin > and buf[128] are both uint8_t*. Correctly written, it's obvious that > only bin is a pointer. Absolutely, auto-formatter got me. > > -case AV_OPT_TYPE_CONST: > > -ret = snprintf(buf, sizeof(buf), "%f", o->default_val.dbl); > > -break; > > I think you should have left the case here. ok > > int av_opt_get_dict_val(void *obj, const char *name, int search_flags, > > AVDictionary **out_val); > > +/** > > Empty line please. done. > > + * Format option value and output to string. > > + * @param[in] val an option value that can be represented as a double. > > + * @param[in] type of the option. > > + * @param[out] out_val value of the option will be written here > > + * @return >=0 on success, a negative error code otherwise > > + * > > + * @note the returned string will be av_malloc()ed and must be av_free()ed > > by the caller > > + */ > > +int av_opt_to_string(double val, enum AVOptionType type, uint8_t > > **out_val); > > If it only works for number-like options, then the name should reflect > it, and the documentation should state it. > > Also, I have reservations about using double here: large integers cannot > be accurately coded with double. I understand your concerns. Perhaps its good to think about the use case (which is more narrow than the current name suggests): av_opt_query_ranges and the device capabilities API return a bunch of AVOptionRange, which contains the fields: double value_min, value_max; I need a function to format these properly (e.g. "yuyv422" instead of 1), and that does not take a AVOption as input, since these min and max values are not stored in an AVOption (and this new function could be used for formatting the values stored in the fields double min and double max in an AVOption as well). Hence the input to the function is double. I agree thats not ideal, nor is it great that values are stored as doubles in a AVOptionRange, since that limits its use to things representable as double (which arguably anything with a range is, logically, but as you said, double can't represent everything). My ideal solution would be to change the following def in AVOption: union { int64_t i64; double dbl; const char *str; /* TODO those are unused now */ AVRational q; } default_val; into a named union, and use that for the default_val of an AVOption, and for AVOptionRange's value_min and value_max. (and possibly also for AVOption's min and max fields, but that may be too big a change). Thats a significant API change, but AVOptionRange is hardly used in the ffmpeg code base (I have no idea about user code, but since they're hardly exposed, i'd expect the same?), but would allow expressing ranges precisely, regardless of type. That would make a more general to_string function possible as well. Anyway, I'd be pretty happy with the solution of just adding a function with a restricted use case and better name. What do you think? Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 20/35] avdevice/avdevice: clean up avdevice_capabilities_create
On Tue, Jun 8, 2021 at 2:09 PM Nicolas George wrote: > > Diederick Niehorster (12021-06-08): > > -*caps = av_mallocz(sizeof(**caps)); > > +*caps = av_mallocz(sizeof(AVDeviceCapabilitiesQuery)); > > var = malloc(sizeof(*var)) is preferred over var = malloc(type), because > if you change the type of var, it happens in another part of the code > and you would have to remember to change it here too. Reverted. Thanks, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 13/35] avdevice: adding control message requesting to show config dialog
On Tue, Jun 8, 2021 at 2:02 PM Nicolas George wrote: > > Diederick Niehorster (12021-06-08): > > +/** > > + * Request to show configuration dialog. > > + * > > + * If device has a configuration dialog of type indicated by > > + * data, show it. > > + * > > + * data: int. > > ? Do i understand correctly that you would like it documented what this int means? It would be avdevice-specific. If a device has multiple dialogs, the int allows indicating which is to be shown. Would adding "data: int: indicates which dialog to show. Meaning is device specific." work? Thanks, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 11/35] avformat: add control_message function to AVInputFormat
On Tue, Jun 8, 2021 at 2:00 PM Nicolas George wrote: > Maybe wrap the commit message at 60 columns. Will do ___ ffmpeg-devel mailing list ffmpeg-devel@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 10/35] fftools: provide media type info for devices
On Tue, Jun 8, 2021 at 1:57 PM Nicolas George wrote: > > The feature looks useful. But the printing must go to stdout, not to > logging, because it is meant to be readable by scripts. Done. Only problem is that now a device of mine that should be called "Microphone Array (Intel® Smart Sound Technology (Intel® SST))" is instead printed as "Intel┬« Smart Sound Technology (Intel┬« SST)". Since devices are selected by name for some avdevices (dshow), thats an issue. Googling tells me that printing UTF-8 (which is whats returned by dshow's get_device_list) with printf is complicated to say the least on Windows. It all works when logging because log.c has implemented win_console_puts(), which looks like it would be easy to make available more broadly. Do you see a solution that would allow printing the device name correctly? Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 26/35] avdevice: Add internal helpers for querying device capabilities
On Tue, Jun 8, 2021 at 3:30 PM Michael Niedermayer wrote: > On Tue, Jun 08, 2021 at 01:04:05AM +0200, Diederick Niehorster wrote: > > brakes build > CC libavdevice/utils.o > libavdevice/utils.c:64:29: error: field ‘query_type’ has incomplete type > enum DshowCapQueryType query_type; > ^~ > libavdevice/utils.c: In function ‘ff_device_get_query_type’: > argh! This slipped through and actually built and worked fine with the msvc-toolchain (implicit int?). That toolchain throws a lot of warnings, didn't spot this one in the noise. Fixed! ___ ffmpeg-devel mailing list ffmpeg-devel@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 05/35] avdevice/dshow: set no-seek flags
didn't have the patchst anymore and already processed comments in later patches, so 02/35 and 05/35 became out of 33. Noticed that too late. Hope that doesn't confuse anything. Else we'll get it on -v2 of this series (which there will certainly be), where i think i'll throttle sending a bit (somehow). On Tue, Jun 8, 2021 at 11:34 AM Valerii Zapodovnikov wrote: > Just resend them (without any -v2). > ___ > ffmpeg-devel mailing list > ffmpeg-devel@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] [discussion] AVOptionRange fields
FWIW, On Sat, Jun 5, 2021 at 11:36 PM Diederick C. Niehorster wrote: > When implementing the avdevice capabilities API, I have run into three > things regarding the AVOptionRange fields (libavutil/opt.h, lines > 310-328) > > 1. an enum AVOptionType field "type" should be added. Else user cannot > know how to interpret the returned value(s), which are all doubles. > NB: for the avdevice capabilities API, option type is an > implementation detail, and cannot be queried by the user. > I turned out to be wrong on this one, the option itself is queryable when the option range is queryable, of course... > 2. a field "is_set" should be added. When querying a range of formats > of an avdevice, sometimes for a given format the queried option is not > available. This is not an error as the user is asking for a valid > capability, it just doesn't always apply to all the matching formats > of the device. This is now communicated through a special value (like > 0 or -1), but that is not ideal. an is_set field would alleviate the > use of special values. > Added in https://ffmpeg.org/pipermail/ffmpeg-devel/2021-June/281123.html > 3. the component_min, component_max fields should be documented > better. "Value's component range" is not informative and the only use > of it in the codebase (av_opt_query_ranges_default() in opt.c, lines > 1848--1918 or the original commit adding it, a8e0d51b, does not help > elucidate what its for). for e.g. an fps, the value range would be > from min_fps to max_fps. What would the associated component range > (min/max) be? > This remains open. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 05/35] avdevice/dshow: set no-seek flags
On Tue, Jun 8, 2021 at 2:13 AM Valerii Zapodovnikov wrote: > Actually I do not know how well will this work. Did you ever play any > stream? Even if you play it without forcing seeking you are allowed to > search forth due to how latency works. That problem with latency was only > fixed in CMAF. ONE must to accelerate decoding forward in time to get low > latency. > > Now hardware devices are different. But still, what about pause? What about > seeking in the end which was BTW broken: see in > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210604062818.2099-1-val.zapod...@gmail.com/ > > I mean, I agree this should be the default. > In its current state, dshow (and from a quick look) many other devices do not support any seeking in either direction. So i think they should be properly flagged as such, at least to get a meaningful error when attempting to seek. By the way, despite using git send-email to send the whole series at once, it seems that they didn't end up in a nice thread (even though headers with in-reply-to were correct), nor did patchworks pick up all the patches (it missed 2/35 [and 5/35], so only attempted to build the first). What could be the issue here, should i throttle send-email somehow for instance? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/35] avdevice/dshow: implement option to use device video timestamps
On Tue, Jun 8, 2021 at 1:29 AM Valerii Zapodovnikov wrote: > Who knows what BS code "TODO figure out math. For now just drop them." > means? Is PTS of the mentioned there can be even theoretically valid or > not? > No, I don't know. I am guessing it refers to wraparound? I also don't understand why a negative time would lead to a large positive value, since a signed integer is used. So i just left this intact. Without a test case/device producing this, there's little that can be done i guess. ___ ffmpeg-devel mailing list ffmpeg-devel@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 24/35] avutil/opt: AVOptionRange gains is_set field.
On Tue, Jun 8, 2021 at 1:45 AM Andreas Rheinhardt < andreas.rheinha...@outlook.com> wrote: > This has absolutely nothing to do with full/limited range, but rather > whether the AVOptionRange contains an interval or a single value. > (Not that I know why this needs to be set explicitly and not implicitly > via is_range = value_min < value_max.) > Yes, the is_range field seems superfluous, but its already there. What are your thoughts on the is_set field? For usage example, see patch device_get_capabilities.c in patch 32/35 in this series. ___ ffmpeg-devel mailing list ffmpeg-devel@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 21/35] avdevice: capabilities API details no longer public
On Tue, Jun 8, 2021 at 1:54 AM Andreas Rheinhardt < andreas.rheinha...@outlook.com> wrote: > Diederick Niehorster: > > NB: will break build, makes needed corresponding changes to avformat. > > > > Then said changes should be part of this patch. Patches should be > logically self-contained and atomic; this is not the same as splitting > by file/library. > > But that is a moot point: James already told you a better way to fix this. > Made sense. But indeed, James' way of doing it obviates that next patch changing avformat.h > > --- a/libavdevice/internal.h > > +++ b/libavdevice/internal.h > > We prefer the order libavutil-libavcodec-libavformat (the reverse of > linking order). > fixed. > > + * AVOption table used by devices to implement device capabilities API. > Should not be used by a user. > > That last part is superfluous, as (external) users by definition have no > business looking into internal.h. > Ah yes, copy paste.. Fixed. > > + */ > > +extern const AVOption av_device_capabilities[]; > > Don't use this name: av_* is our namespace for public symbols. Our > linker script makes sure that all symbols that start with av_* are > exported; whether they are in a public header is irrelevant. > Not exporting the symbol in the first place also solves the problem of > users using it despite it not being part of the API. > (I btw don't know whether said linker script is used on Windows.) > I changed it to a ff_ prefix. Is that correct? ___ ffmpeg-devel mailing list ffmpeg-devel@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 21/35] avdevice: capabilities API details no longer public
On Tue, Jun 8, 2021 at 1:26 AM James Almer wrote: > On 6/7/2021 8:04 PM, Diederick Niehorster wrote: > Instead of removing the struct altogether, just keep its typedef here. > That way the functions below and any libavformat struct can still use > AVDeviceCapabilitiesQuery as an incomplete type. > > So in short: > > typedef struct AVDeviceCapabilitiesQuery AVDeviceCapabilitiesQuery; > Will do. > This is nonetheless technically an API break, but since nothing used > this stuff, i guess we could make an exception. Not sure what other > developers think. > That was my thinking, and I see Andreas agrees with this too, good. > > I assume you intend to also undo the deprecation? > Yes, thats patch 19/35 in this series. ___ ffmpeg-devel mailing list ffmpeg-devel@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] libavcodec: r12b decoder
>From a distance, the patch looks like its in the correct format and everything (generated by git format-patch, right?). Is it against current HEAD? There may be a conflict On Mon, Jun 7, 2021 at 8:02 AM Dennis Fleurbaaij wrote: > > Failed to apply, what is the exact way you want the patch? > > Kind regards, > > Dennis Fleurbaaij > +31 (0) 6 54 21 5365 > > > On Sat, Jun 5, 2021 at 6:12 PM Valerii Zapodovnikov > wrote: > > > That did work, cool. > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/capucmebbw-rkt3mw5berkg3cqa+-akryfahclfc36mh2ybn...@mail.gmail.com/ > > > > BTW, will have to look into patchwork's patchwork (D:)) whether they fixed > > .patch extensions. Apparently not. Also everything in "Re:" cannot contain > > a patch. Ha. > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4] avdevice/dshow: implement capabilities API
On Sun, Jun 6, 2021 at 6:15 AM Andreas Rheinhardt wrote: > Diederick C. Niehorster: > > See other mails: the bit of documentation in avdevice.h (lines > > 334--402) suggest the capabilities API is supposed to be used on an > > unopened device. It would make sense: probe the device for what it can > > do, decide what settings you want to use, then open device). In case > > of the dshow device, I think i cannot avoid potential false positives > > (entries in the query output that shouldn't be there) if the device is > > already opened. I'll experiment, perhaps this is only a theoretical > > worry. > > > And is the AVFormatContext that has been used for probing then supposed > to be reused for actually using the device? (You are calling > dshow_read_close() manually, but our cleanup functions typically only > clean up things that need to be freed; they e.g. don't reset (say) > ordinary ints to zero, although the code may rely upon this.) It is not supposed to, but does work. However, as user you still need to set the device options in the opts dict passed to avformat_open_input() even after narrowing down the format you want to use through the capabilities API. I'll document this. NB: reusing the AVFormatContext currently leaks a little bit of memory as, e.g., avformat_open_input() set url without checking and freeing if its already set. And maybe more. I'll just add a patch with my proposed API extension avformat_alloc_input_context() and needed extra checks to avformat_open_input(), so we can discuss it. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] [discussion] AVOptionRange fields
When implementing the avdevice capabilities API, I have run into three things regarding the AVOptionRange fields (libavutil/opt.h, lines 310-328) 1. an enum AVOptionType field "type" should be added. Else user cannot know how to interpret the returned value(s), which are all doubles. NB: for the avdevice capabilities API, option type is an implementation detail, and cannot be queried by the user. 2. a field "is_set" should be added. When querying a range of formats of an avdevice, sometimes for a given format the queried option is not available. This is not an error as the user is asking for a valid capability, it just doesn't always apply to all the matching formats of the device. This is now communicated through a special value (like 0 or -1), but that is not ideal. an is_set field would alleviate the use of special values. 3. the component_min, component_max fields should be documented better. "Value's component range" is not informative and the only use of it in the codebase (av_opt_query_ranges_default() in opt.c, lines 1848--1918 or the original commit adding it, a8e0d51b, does not help elucidate what its for). for e.g. an fps, the value range would be from min_fps to max_fps. What would the associated component range (min/max) be? Any thoughts before i add the two fields suggested fields? Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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/4] avdevice/dshow: implement capabilities API
On Sat, Jun 5, 2021 at 2:25 PM Andreas Rheinhardt wrote: > Diederick Niehorster: > > This implements avdevice_capabilities_create and avdevice_capabilities_free > > for the dshow device.> > + > > +if (ranges) { > > +if (query_type == CAP_QUERY_FRAME_SIZE) { > > +for (int j = 0; j < 3; j++) { > > +range = av_mallocz(sizeof(AVOptionRange)); > > It seems that you are allocating individual AVOptionRange structures. > This is bad, because that is not how av_opt_freep_ranges() treats them: > They are supposed to be pointers into one big array of AVOptionRange > structures. av_opt_freep_ranges() does this (NB: ranges->range is an AVOptionRange **): for (i = 0; i < ranges->nb_ranges * ranges->nb_components; i++) { AVOptionRange *range = ranges->range[i]; if (range) { av_freep(&range->str); av_freep(&ranges->range[i]); // <-- } } av_freep(&ranges->range); Note line highlighted by <--: it frees each individual element, one at a time, and only after that deletes the whole array. So i think it is correct that i am allocating them one at a time? Am i misreading/-understanding this? > > +const AVFormatContext *avctx = caps->device_context; > > avctx is only used for AVCodecContexts by convention. the dshow device (almost) consistently uses avctx as name for the AVFormatContext. Should i change that everywhere (what is the convention name for a AVFormatContext?), or is it ok if i remain consistent with that here? > > +if (ctx->device_name[0]) { > > +av_log(avctx, AV_LOG_ERROR, "You cannot query device capabilities > > on an opened device\n"); > > Is this supposed to be a limitation of the capabilities API in general > or only of the implementation here in for dshow? See other mails: the bit of documentation in avdevice.h (lines 334--402) suggest the capabilities API is supposed to be used on an unopened device. It would make sense: probe the device for what it can do, decide what settings you want to use, then open device). In case of the dshow device, I think i cannot avoid potential false positives (entries in the query output that shouldn't be there) if the device is already opened. I'll experiment, perhaps this is only a theoretical worry. Thanks! Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/4] avdevice/dshow: implement capabilities API
On Sat, Jun 5, 2021 at 4:36 PM Anton Khirnov wrote: > Sorry to rain on your parade, but I don't think we should go ahead with > this before deciding what is to be done with libavdevice. The last > discussion about it died without being resolved, but the issues are > still present. I understand. I realize I'm new here: Is there a timeline for such a discussion and could the discussion benefit from a real usage example such as this patch series? I guess a big change in libavdevice should at least offer the functionality the current implementation offers (or theoretically offers :p). I really like the current design where an avdevice can just be used through the avformat interface. Add avdevice_register_all(); and Bob's your uncle! To therefore argue a counterpoint, especially if the timeline is long: accepting an implementation of the device capabilities API for an avdevice will help steer that discussion, ensuring this capability is not forgotten, and also resolves a real usage need (well, just mine) now instead of in some possibly distant future. In the meantime, I'll fix up my patch with Andreas' suggestions, along with proposed changes for cleaning up the public avdevice capabilities API if undeprecated, so its all ready to go if the decision goes in favor. All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 4/4] examples: adding device_get_capabilities example
On Sat, Jun 5, 2021 at 2:41 PM Andreas Rheinhardt wrote: > > Have you looked at > 2ff40b98ecbd9befadddc8fe665a391f99bfca32/0a071f7124beaf0929f772a8618ac1b6c17b0222? I have, but didn't know what to do with it. Are you suggesting reverting part of those two commits? The bit of documentation of the API (lines 334--402) of avdevice.h suggest it was the intention for the API to work with an allocated, but not opened, input or output context and at the time that was written, it was possible to create either. Now, as you know, input formats can only be directly opened and created. It seems the removed flag and such you are referring to in the commits had another use, and i don't see that a new flag is needed. If i would implement something like avformat_alloc_input_context, we: 1. would again be able to make allocated but unopened iformats (admittedly for the very small use case of using this API, but at no cost if used in other cases 2. would only need to make small changes to avformat_open_input (don't set url if input NULL, else overwrite??; don't allocate priv_data if already set; think thats it) So no need for adding a flag, or reintroducing demuxer_open or so, i think. > > [...] > > > Of course you can propose all sorts of API extensions; but don't be too > beholden to the current API. It has never been tested in practice and it > shows (e.g. it is built on top of the AVOptionRanges API which is quite > underdocumented (see the remark/gotcha about the AVOptionRange structs > not being individually allocated for your patch 3/4); and making each > query_range callback allocate an AVOptionRange unnecessarily ties > sizeof(AVOptionRange) to the ABI -- I don't get why this has been done). > Also what you are experiencing here shows that there probably never was > a working example. Yes, I had trouble wrapping my head around the AVOptionRanges API, the only usage example i found was av_opt_query_ranges_default() in libavutil/opt.c and regarding the gotcha, it seems i misread/misunderstood something of what i read there. Do you think making sizeof(AVOptionRange) part of the public ABI is something to fix / can it still be fixed? > Besides that, there are more weird things: Why is > AVDeviceCapabilitiesQuery a public struct and not an opaque one if it is > not to be used? Why is av_device_capabilities public if it should not be > used by a user? Lets clean it up. And since it hasn't been used yet, i guess i can do so without deprecations for either? Just to check, you are suggesting the following, correct? 1. from avdevice.h, remove typedef struct AVDeviceCapabilitiesQuery and extern const AVOption av_device_capabilities[]; 2. change to avdevice_capabilities_create and avdevice_capabilities_free signatures to taking **void as first input 3. remove forward declare of struct AVDeviceCapabilitiesQuery; from avformat.h 4. change signature of create_device_capabilities and free_device_capabilities to taking *void as second argument Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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 4/4] examples: adding device_get_capabilities example
On Sat, Jun 5, 2021 at 8:51 AM Andreas Rheinhardt wrote: > > Diederick Niehorster: > > + > > +// since there is no equivalent of avformat_alloc_output_context2 for > > an input context, > > +// so we get this dirty code that users shouldn't have to write > > +fmt_ctx = avformat_alloc_context(); > > +fmt_ctx->url = av_strdup(dshow_input_name); > > Missing checks. > > > +fmt_ctx->iformat = fmt; > > +if (fmt_ctx->iformat->priv_data_size > 0) { > > priv_data_size is not part of the public API; so the code is indeed > dirty and so this example is unacceptable as-is. This might be a > scenario where AVFMT_FLAG_PRIV_OPT might have been useful, see > 2ff40b98ecbd9befadddc8fe665a391f99bfca32. > > > +if (!(fmt_ctx->priv_data = > > av_mallocz(fmt_ctx->iformat->priv_data_size))) { > > +ret = AVERROR(ENOMEM); > > +goto end; > > +} > > +if (fmt_ctx->iformat->priv_class) { > > +*(const AVClass**)fmt_ctx->priv_data = > > fmt_ctx->iformat->priv_class; > > +av_opt_set_defaults(fmt_ctx->priv_data); > > +} > > +} > > +// end dirty code I have fixed the rest, but I believe it is not possible to fix the above problem with the current API? The code above shows the state i need the format context and attached input context to be in when calling av_opt_query_ranges. The device cannot be opened (that is read_header should not be called) before calling av_opt_query_ranges, as it is not possible to reliably query possible settings of the device once it is opened (without temporarily closing it anyway, which would be undesirable). One solution i see is to: 1. implement a avformat_alloc_input_context() 2. analogous to avformat_alloc_output_context2, add a few if-statements in avformat_open_input() to skip steps if they are already done on the passed-in AVFormatContext or its iformat. It would e.g. allow something along the lines of: AVFormatContext* fmt_ctx = NULL; avformat_alloc_input_context(&fmt_ctx, NULL, "dshow", "video=\"Integrated Webcam\""); // ... // query device capabilities, built up option dict for setting up device as wanted based on query results // avformat_open_input(&fmt_ctx,NULL,NULL,opts); // NB: filename and fmt inputs above are NULL as already set by avformat_alloc_input_context Would such an API extension be considered? NB: The constraint of my implementation of the AVDevice Capabilties API should also be documented. I propose, more generally: the avdevice_capabilities_create/av_opt_query_ranges API has the constraint that while it is always valid to query capabilities on an unopened avdevice (as provided by avformat_alloc_input_context), some devices may not allow querying capabilities once avformat_open_input() has been called on them. In that case, upon av_opt_query_ranges these devices will return AVERROR(EIO). All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] avdevice/avdevice: Deprecate AVDevice Capabilities API
Hi All, If instead of the various separate patch series i have sent the last few days, you would like to see one integrated series where all are applied on top of each other and conflicts resolves, please see all the commits ahead of master (currently 22) on the develop branch here: https://github.com/dcnieho/ffmpeg/tree/develop In making this branch, i have also done some small refactoring, and fixed some issues i came across in my implementation. As said before, I hope one of you can advise me on how to submit all this (one big series so it all applies cleanly?). The brunt is letting dshow provide more information about what video data it provides, and especially making it way more controllable programmatically/through the API. It is now at a point where i can use ffmpeg/dshow as a backend to flexible, reconfigurable use of e.g. a webcam. Exciting to me:) All the best, Dee On Fri, Jun 4, 2021 at 2:22 PM Diederick C. Niehorster wrote: > > Hi Nicolas, > > On Fri, Jun 4, 2021 at 11:06 AM Nicolas George wrote: > > I do not understand: you did send them as a large patch series. Twice, > > by the way, which is confusing. > > Yes, the first series got messed up, send it a second time correctly. > I've cleaned up patchwork, it only shows the right ones. > > I have sent multiple patch series, all of which apply cleanly to > master, but some of which heavily conflict with each other. E.g. > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4090, > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4088 and > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4100 > implement different features, but each heavily edit overlapping code. > Once one is merged, the others will conflict. Should i put these all > in one large series implementing multiple features? > > > It is the way to do it: make clean commits in your Git tree, using > > rebase if necessary to make them really clean. Then let git format-patch > > or send-email make a patch series, with each patch stacking on top of > > the other. > > Thanks! send-email I have made some mistakes with my first times here, > sorry again for the double patches. > > Cheers, > Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] avdevice/avdevice: Deprecate AVDevice Capabilities API
Hi Nicolas, On Fri, Jun 4, 2021 at 11:06 AM Nicolas George wrote: > I do not understand: you did send them as a large patch series. Twice, > by the way, which is confusing. Yes, the first series got messed up, send it a second time correctly. I've cleaned up patchwork, it only shows the right ones. I have sent multiple patch series, all of which apply cleanly to master, but some of which heavily conflict with each other. E.g. https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4090, https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4088 and https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4100 implement different features, but each heavily edit overlapping code. Once one is merged, the others will conflict. Should i put these all in one large series implementing multiple features? > It is the way to do it: make clean commits in your Git tree, using > rebase if necessary to make them really clean. Then let git format-patch > or send-email make a patch series, with each patch stacking on top of > the other. Thanks! send-email I have made some mistakes with my first times here, sorry again for the double patches. Cheers, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] avdevice/avdevice: Deprecate AVDevice Capabilities API
Hi Nicolas, On Wed, Jun 2, 2021 at 2:37 PM Nicolas George wrote: > Excellent. Applications that use the advanced features of libavdevice > and serve as test beds for these features are sorely needed. > > The project has no real system to make engagements about something like > this, but I can say that if you propose a patch series that de-deprecate > the API and implements it in dshow, I would personally support it. Just sent the patch, it completes my push together with my other patches of the last few days to make the dshow device fully programmatically controllable and discoverable. By the way, each of these patch series applies to master, but not on top of each other (there'd be massive conflicts). What is custom? Should i instead send them all as one large patch series? Thanks and all the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@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] avdevice/avdevice: Deprecate AVDevice Capabilities API
On Wed, Jun 2, 2021 at 2:37 PM Nicolas George wrote: > > dcni...@gmail.com (12021-06-02): > > I (new contributor) have been making a push to get avdevice/dshow to where > > it is great to use programmatically (see recent patches, more coming). One > > thing on my list is to be able to through the API query the capabilities of > > my device, as right now I'd have to ask users to run a command like ffmpeg > > -list_options true -f dshow -i video="Camera" to get that info and manually > > enter it into my program, let alone that I am unable to make a nicer GUI. I > > see that this capability has just been deprecated. Instead of waiting for a > > new avdevice interface that may or may not come (I personally like that they > > are just formats like anything else, super easy to use), would it be > > possible to undeprecate when I provide an implementation of > > create_device_capabilities for the dshow avdevice? > > Excellent. Applications that use the advanced features of libavdevice > and serve as test beds for these features are sorely needed. > > The project has no real system to make engagements about something like > this, but I can say that if you propose a patch series that de-deprecate > the API and implements it in dshow, I would personally support it. Great to hear! What should i do to de-deprecate? 1. bump LIBAVDEVICE_VERSION_MINOR 2. remove FF_API_DEVICE_CAPABILITIES define 3. remove the #if LIBAVFORMAT_VERSION_MAJOR < 59 in avformat.h around int (*create_device_capabilities)(struct AVFormatContext *s, struct AVDeviceCapabilitiesQuery *caps); and int (*free_device_capabilities)(struct AVFormatContext *s, struct AVDeviceCapabilitiesQuery *caps); ? All the best, Dee ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".