Re: [FFmpeg-devel] FFmpeg 7.0 blocking issues

[Kacper Michajlow]

On Mon, 3 Jun 2024 at 23:41, James Almer  wrote:
>
> On 6/3/2024 6:32 PM, Michael Niedermayer wrote:
> > On Sun, Jun 02, 2024 at 03:49:42PM +0200, Sebastian Ramacher wrote:
> >> On 2024-03-03 09:55:15 +0100, Sebastian Ramacher wrote:
> >>> On 2024-03-02 20:39:08 -0500, Sean McGovern wrote:
>  On Sat, Mar 2, 2024, 18:19 Michael Niedermayer 
>  wrote:
> 
> > On Sun, Mar 03, 2024 at 12:06:14AM +0100, Sebastian Ramacher wrote:
> >> On 2024-03-02 23:55:38 +0100, Michael Niedermayer wrote:
> >>> On Tue, Jan 23, 2024 at 08:22:41PM +0100, Michael Niedermayer wrote:
>  Hi all
> 
>  As it was a little difficult for me to not loose track of what is
>  blocking a release. I suggest that for all release blocking issues
>  open a ticket and set Blocking to 7.0
>  that way this:
>  https://trac.ffmpeg.org/query?blocking=~7.0
> 
>  or for the ones not closed:
> 
> > https://trac.ffmpeg.org/query?status=new=open=reopened=~7.0
> 
>  will list all blocking issues
> 
>  Ive added one, for testing that, i intend to add more if i see
> > something
> 
>  What is blocking? (IMHO)
>  * regressions (unless its non possible to fix before release)
>  * crashes
>  * security issues
>  * data loss
>  * privacy issues
>  * anything the commuity agrees should be in the release
> >>>
> >>> We still have 3 blocking issues on trac
> >>>
> >>> do people want me to wait or ignore them and branch ?
> >>> Iam not sure when the exact deadline is but if we keep waiting
> >>> we will not get into ubuntu 24.04 LTS
> >>
> >> 24.04 is past feature freeze, so it's too late for that.
> >
> > we should aim earlier in the future then.
> >
> >
> 
>  LTS is only every 2 years, yes?
> >>>
> >>> Yes
> >>>
>  How do we make sure this doesn't happen in 2026? How much of a gap is 
>  there
>  between feature freeze and release?
> >>>
> >>> Not involved in Ubuntu, so that's from past experience: feature
> >>> freeze is usually about two months before the release.
> >>>
> >>> So here's the catch: Debian's timeline also needs to be taken into
> >>> account. If the ffmpeg release does not involve the removal of deprecated 
> >>> API and
> >>> a SONAME bump, then the time from ffmpeg to release to upload to Debian
> >>> unstable and then import in Ubuntu is short. In this case, I am sure
> >>> that I could convince Ubuntu maintainers to import it even during
> >>> feature freeze.
> >>>
> >>> But with SONAME bumps and changes in the API, it takes a lot more time
> >>> to work through the high number of ffmpeg reverse dependencies. In that
> >>> case, plan a release at least 6 months before an Ubuntu LTS release.
> >>
> >>
> >>
> >>>
> >>> We usually have to rely on upstream maintainers to adopt to the
> >>> changes and that take times. Many moons ago Anton helped with providing
> >>> patches, but for the last couple of API changes it took some months from
> >>> "dear maintainer, here is ffmpeg X for testing, please fix the build of
> >>> your package" to actually doing all uploads and rebuilds. For example,
> >>> the transition to ffmpeg 6.0 was started in July 2023 and was done in
> >>> December 2023.
> >>
> >> Just as a FYI: ffmpeg 7.0 breaks close to 70 reverse dependencies in
> >> Debian. The list is available at [1]. So if you want ffmpeg X to be in
> >> Debian Y or Ubuntu Z, X needs to be released at least half a year before
> >> Y or Z freeze.
> >
> > Is there something that ffmpeg can do to reduce this breakage ?
> > (i know its a bit of a lame question as its API brekages but i mean
> > can the policy we have about deprecating API/ABI be amended in some way
> > to make this easier ?
>
> Well, no. Breakages are expected when you remove API. The real question
> is why so many projects wait until the old API is gone to migrate. What
> we removed in 7.0 has had its replacement in place for a couple years,
> since 5.1.

Is it the real question, though? I think it is unrealistic to expect
every single developer to track FFmpeg changes and immediately adjust
their projects to the latest API. Not only can this time be allocated
in different areas, but any change also introduces an additional risk
of breakage. Until the old API stops working, it is really up to
individual project maintainers to decide when they want to upgrade. If
they choose to upgrade in bulk after the new stable version is
released, it is perfectly understandable.

Even if a project is updated to a new API, as in the case of mpv, we
still experienced breakage after the removal of the old API. One build
[1] and one runtime [2] issue occurred simply because the deprecation
itself was not sufficiently visible, or not visible at all, and those
two small pieces were missing. Sure, you can blame developers for not
reading the API 

Re: [FFmpeg-devel] [PATCH] avcodec/vp9mvs: fix misaligned access when clearing VP9mv

[Kacper Michajlow]

On Sun, 2 Jun 2024 at 23:17, Ronald S. Bultje  wrote:
>
> Hi,
>
> On Sun, Jun 2, 2024 at 9:12 AM James Almer  wrote:
>
> > On 6/2/2024 10:06 AM, James Almer wrote:
> > > On 6/2/2024 9:14 AM, Kacper Michajłow wrote:
> > >> Fixes runtime error: member access within misaligned address
> > >>  for type 'av_alias64', which requires 8 byte alignment.
> > >>
> > >> VP9mv is aligned to 4 bytes, so instead doing 8 bytes clear, let's do
> > >> 2 times 4 bytes.
> > >>
> > >> Signed-off-by: Kacper Michajłow 
> > >> ---
> > >>   libavcodec/vp9mvs.c | 3 ++-
> > >>   1 file changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/libavcodec/vp9mvs.c b/libavcodec/vp9mvs.c
> > >> index b706d1660f..790cf629a6 100644
> > >> --- a/libavcodec/vp9mvs.c
> > >> +++ b/libavcodec/vp9mvs.c
> > >> @@ -294,7 +294,8 @@ void ff_vp9_fill_mv(VP9TileData *td, VP9mv *mv,
> > >> int mode, int sb)
> > >>   VP9Block *b = td->b;
> > >>   if (mode == ZEROMV) {
> > >> -AV_ZERO64(mv);
> > >> +AV_ZERO32([0]);
> > >> +AV_ZERO32([1]);
> > >>   } else {
> > >>   int hp;
> > >
> > > IMO just move mv in VP9Block to the top of the struct. That will make
> > > sure it's aligned to at the very least 16 byte (Since it's av_malloc'd).
> >
> > Actually nevermind, VP9mv has two int16_t and given what's passed to
> > ff_vp9_fill_mv() it's not enough.
> >
>
> Do compilers on relevant platforms convert this to a single 64bit
> (unaligned) zero-move? Otherwise, we may want an unaligned AV_ZERO64() so
> as to not slow down platforms supporting unaligned writes.

Yes, exactly, compilers do that. I've checked before sending this
patch if it doesn't do something overly silly.

You can play around here to see, I've extracted relevant part
https://godbolt.org/z/K4d7Ejb1P

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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/6] avformat/matroskadec: export cropping values

[Kacper Michajlow]

On Wed, 29 May 2024 at 23:47, James Almer  wrote:
>
> Signed-off-by: James Almer 
> ---
>  libavformat/matroskadec.c | 53 +++
>  1 file changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 2f07e11d87..a30bac786b 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -213,7 +213,13 @@ typedef struct MatroskaTrackVideo {
>  uint64_t display_height;
>  uint64_t pixel_width;
>  uint64_t pixel_height;
> +uint64_t cropped_width;
> +uint64_t cropped_height;
>  EbmlBin  color_space;
> +uint64_t pixel_cropt;
> +uint64_t pixel_cropl;
> +uint64_t pixel_cropb;
> +uint64_t pixel_cropr;
>  uint64_t display_unit;
>  uint64_t interlaced;
>  uint64_t field_order;
> @@ -527,10 +533,10 @@ static EbmlSyntax matroska_track_video[] = {
>  { MATROSKA_ID_VIDEOALPHAMODE,  EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, alpha_mode), { .u = 0 } },
>  { MATROSKA_ID_VIDEOCOLOR,  EBML_NEST,  0, 
> sizeof(MatroskaTrackVideoColor), offsetof(MatroskaTrackVideo, color), { .n = 
> matroska_track_video_color } },
>  { MATROSKA_ID_VIDEOPROJECTION, EBML_NEST,  0, 0, 
> offsetof(MatroskaTrackVideo, projection), { .n = 
> matroska_track_video_projection } },
> -{ MATROSKA_ID_VIDEOPIXELCROPB, EBML_NONE },
> -{ MATROSKA_ID_VIDEOPIXELCROPT, EBML_NONE },
> -{ MATROSKA_ID_VIDEOPIXELCROPL, EBML_NONE },
> -{ MATROSKA_ID_VIDEOPIXELCROPR, EBML_NONE },
> +{ MATROSKA_ID_VIDEOPIXELCROPB, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, pixel_cropb), {.u = 0 } },
> +{ MATROSKA_ID_VIDEOPIXELCROPT, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, pixel_cropt), {.u = 0 } },
> +{ MATROSKA_ID_VIDEOPIXELCROPL, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, pixel_cropl), {.u = 0 } },
> +{ MATROSKA_ID_VIDEOPIXELCROPR, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, pixel_cropr), {.u = 0 } },
>  { MATROSKA_ID_VIDEODISPLAYUNIT,EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, display_unit), { .u= 
> MATROSKA_VIDEO_DISPLAYUNIT_PIXELS } },
>  { MATROSKA_ID_VIDEOFLAGINTERLACED, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, interlaced),  { .u = 
> MATROSKA_VIDEO_INTERLACE_FLAG_UNDETERMINED } },
>  { MATROSKA_ID_VIDEOFIELDORDER, EBML_UINT,  0, 0, 
> offsetof(MatroskaTrackVideo, field_order), { .u = 
> MATROSKA_VIDEO_FIELDORDER_UNDETERMINED } },
> @@ -2963,14 +2969,30 @@ static int mkv_parse_video(MatroskaTrack *track, 
> AVStream *st,
>
>  if (track->video.display_unit < MATROSKA_VIDEO_DISPLAYUNIT_UNKNOWN) {
>  if (track->video.display_width && track->video.display_height &&
> -par->height  < INT64_MAX / track->video.display_width  / 
> display_width_mul &&
> -par->width   < INT64_MAX / track->video.display_height / 
> display_height_mul)
> +track->video.cropped_height < INT64_MAX / 
> track->video.display_width  / display_width_mul &&
> +track->video.cropped_width  < INT64_MAX / 
> track->video.display_height / display_height_mul)
>  av_reduce(>sample_aspect_ratio.num,
>>sample_aspect_ratio.den,
> -  par->height * track->video.display_width  * 
> display_width_mul,
> -  par->width  * track->video.display_height * 
> display_height_mul,
> +  track->video.cropped_height * 
> track->video.display_width  * display_width_mul,
> +  track->video.cropped_width  * 
> track->video.display_height * display_height_mul,
>INT_MAX);
>  }
> +if (track->video.cropped_width  != track->video.pixel_width ||
> +track->video.cropped_height != track->video.pixel_height) {
> +uint8_t *cropping;
> +AVPacketSideData *sd = 
> av_packet_side_data_new(>codecpar->coded_side_data,
> +   
> >codecpar->nb_coded_side_data,
> +   
> AV_PKT_DATA_FRAME_CROPPING,
> +   sizeof(uint32_t) * 4, 
> 0);
> +if (!sd)
> +return AVERROR(ENOMEM);
> +
> +cropping = sd->data;
> +bytestream_put_le32(, track->video.pixel_cropt);
> +bytestream_put_le32(, track->video.pixel_cropb);
> +bytestream_put_le32(, track->video.pixel_cropl);
> +bytestream_put_le32(, track->video.pixel_cropr);
> +}
>  if (par->codec_id != AV_CODEC_ID_HEVC)
>  sti->need_parsing = AVSTREAM_PARSE_HEADERS;
>
> @@ -3136,10 +3158,21 @@ static int matroska_parse_tracks(AVFormatContext *s)
>  track->default_duration = default_duration;
>  }
>  }
> +track->video.cropped_width  = track->video.pixel_width;
> +

Re: [FFmpeg-devel] [PATCH] avcodec/h2645_sei: loosen up min luminance requirements

[Kacper Michajlow]

On Sat, 25 May 2024 at 13:36, Niklas Haas  wrote:
>
> From: Niklas Haas 
>
> The H.265 specification is quite clear on this case:
>
> > When min_display_mastering_luminance is not in the range of 1 to
> > 5, the nominal maximum display luminance of the mastering display
> > is unknown or unspecified or specified by other means not specified in
> > this Specification.
>
> And so the current code is correct in marking luminance data as invalid
> if min luminance is set to 0. However, this breaks playback of at least
> several real-world Blu-ray releases, for example La La Land, Planet of
> the Apes, and quite possibly a lot more. These come with ostensibly
> valid max_luminance tags (1000 nits), but min_luminance set to 0.
>
> Loosen up this requirement by guarding it behind FF_COMPLIANCE_STRICT.
> We still reject blatantly invalid metadata (wrong value range on
> luminance, max set to 0, max below min, min above 50 nits etc.), so this
> shouldn't cause any unintended regressions.
>
> Fixes: https://github.com/mpv-player/mpv/issues/14177
> ---
>  libavcodec/h2645_sei.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index 1deb76c765..7c83747cd0 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -619,11 +619,15 @@ static int h2645_sei_to_side_data(AVCodecContext 
> *avctx, H2645SEI *sei,
>
>  metadata->min_luminance.num = 
> sei->mastering_display.min_luminance;
>  metadata->min_luminance.den = luma_den;
> -metadata->has_luminance &= sei->mastering_display.min_luminance 
> >= 1 &&
> -   sei->mastering_display.min_luminance 
> <= 5 &&
> +metadata->has_luminance &= sei->mastering_display.min_luminance 
> <= 5 &&
> sei->mastering_display.min_luminance <
> sei->mastering_display.max_luminance;
>
> +/* Real (blu-ray) releases in the wild come with minimum 
> luminance
> + * values of 0.000 cd/m2, so permit this edge case */
> +if (avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT)
> +metadata->has_luminance &= 
> sei->mastering_display.min_luminance >= 1;
> +
>  if (metadata->has_luminance || metadata->has_primaries)
>  av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
>  if (metadata->has_primaries) {
> --
> 2.45.0

LGTM

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/data_uri: Fix base64 decode buffer size calculation

[Kacper Michajlow]

On Sat, 11 May 2024 at 03:45, Michael Niedermayer
 wrote:
>
> On Thu, May 09, 2024 at 04:02:09PM +0200, Kacper Michajłow wrote:
> > Also reject input if it is too short.
> >
> > Found by OSS-Fuzz.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> >  libavformat/data_uri.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/data_uri.c b/libavformat/data_uri.c
> > index 3868a19630..f97ecbab37 100644
> > --- a/libavformat/data_uri.c
> > +++ b/libavformat/data_uri.c
> > @@ -73,11 +73,11 @@ static av_cold int data_open(URLContext *h, const char 
> > *uri, int flags)
> >  data++;
> >  in_size = strlen(data);
> >  if (base64) {
> > -size_t out_size = 3 * (in_size / 4) + 1;
> > +size_t out_size = AV_BASE64_DECODE_SIZE(in_size);
>
> i suspect this is correct
>
>
> >
> >  if (out_size > INT_MAX || !(ddata = av_malloc(out_size)))
> >  return AVERROR(ENOMEM);
>
> > -if ((ret = av_base64_decode(ddata, data, out_size)) < 0) {
> > +if (!out_size || (ret = av_base64_decode(ddata, data, out_size)) < 
> > 0) {
> >  av_free(ddata);
> >  av_log(h, AV_LOG_ERROR, "Invalid base64 in URI\n");
> >  return ret;
>
> why would this need a out_size == 0 check ?
>
> also it seems av_base64_decode() itself is buggy, ive sent 2 patches
> fixing av_base64_decode() and extening the self tests

Thank you for the fix. I can confirm it works. I've still sent an
AV_BASE64_DECODE_SIZE change, just for the correctness of it.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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/vulkan_decode: Make Vulkan decode less spammy in verbose logs

[Kacper Michajlow]

On Wed, 7 Feb 2024 at 03:47, Lynne  wrote:
>
> Feb 7, 2024, 03:11 by kaspe...@gmail.com:
>
> > Drop per frame decode messages to AV_LOG_TRACE level.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> >  libavcodec/vulkan_av1.c  | 2 +-
> >  libavcodec/vulkan_h264.c | 2 +-
> >  libavcodec/vulkan_hevc.c | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/vulkan_av1.c b/libavcodec/vulkan_av1.c
> > index 9730e4b08d..c7d5a54012 100644
> > --- a/libavcodec/vulkan_av1.c
> > +++ b/libavcodec/vulkan_av1.c
> > @@ -530,7 +530,7 @@ static int vk_av1_end_frame(AVCodecContext *avctx)
> >  rav[i] = ap->ref_src[i]->f;
> >  }
> >
> > -av_log(avctx, AV_LOG_VERBOSE, "Decoding frame, %"SIZE_SPECIFIER" 
> > bytes, %i tiles\n",
> > +av_log(avctx, AV_LOG_TRACE, "Decoding frame, %"SIZE_SPECIFIER" bytes, 
> > %i tiles\n",
> >  vp->slices_size, ap->tile_list.nb_tiles);
> >
> >  return ff_vk_decode_frame(avctx, pic->f, vp, rav, rvp);
> > diff --git a/libavcodec/vulkan_h264.c b/libavcodec/vulkan_h264.c
> > index 39c123ddca..c918dbaa13 100644
> > --- a/libavcodec/vulkan_h264.c
> > +++ b/libavcodec/vulkan_h264.c
> > @@ -529,7 +529,7 @@ static int vk_h264_end_frame(AVCodecContext *avctx)
> >  rav[i] = hp->ref_src[i]->f;
> >  }
> >
> > -av_log(avctx, AV_LOG_VERBOSE, "Decoding frame, %"SIZE_SPECIFIER" 
> > bytes, %i slices\n",
> > +av_log(avctx, AV_LOG_TRACE, "Decoding frame, %"SIZE_SPECIFIER" bytes, 
> > %i slices\n",
> >  vp->slices_size, hp->h264_pic_info.sliceCount);
> >
> >  return ff_vk_decode_frame(avctx, pic->f, vp, rav, rvp);
> > diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c
> > index 033172cbd6..0f6f2e775b 100644
> > --- a/libavcodec/vulkan_hevc.c
> > +++ b/libavcodec/vulkan_hevc.c
> > @@ -903,7 +903,7 @@ static int vk_hevc_end_frame(AVCodecContext *avctx)
> >  rvp[i] = >vp;
> >  }
> >
> > -av_log(avctx, AV_LOG_VERBOSE, "Decoding frame, %"SIZE_SPECIFIER" 
> > bytes, %i slices\n",
> > +av_log(avctx, AV_LOG_TRACE, "Decoding frame, %"SIZE_SPECIFIER" bytes, 
> > %i slices\n",
> >  vp->slices_size, hp->h265_pic_info.sliceSegmentCount);
> >
> >  return ff_vk_decode_frame(avctx, pic->frame, vp, rav, rvp);
> >
>
> I don't agree with this, without this print,
> you have no idea if hardware decoding is even being used.

With this print you have no idea either. Unless you know only Vulkan
decoder prints those messages, (that alone is a reason to remove
them), there is nothing indicative about Vulkan or even hardware
decoding in those messages, AVClass item_name is just "h264".

[ffmpeg/video] h264: Decoding frame, 15808 bytes, 1 slices
[ffmpeg/video] h264: Decoding frame, 17203 bytes, 1 slices
[ffmpeg/video] h264: Decoding frame, 16862 bytes, 1 slices
[ffmpeg/video] h264: Decoding frame, 15989 bytes, 1 slices

Also debugging if something like hw decoding is working is precisely
AV_LOG_TRACE or AV_LOG_DEBUG, not AV_LOG_VERBOSE, which in my opinion
should be still useful information for the end user, not only for
tracing purposes. "Using Vulkan hwdec on device foo" can be printed
once at the beginning of decode... "Decoding frame" messages are in
general not useful at all.

> MP4 parsing is far too noisy than a single print per frame.

"They are bad, so I can be bad too." is non-argument. Also I don't see
exactly what you are referring to, in my experience MP4 is less noisy
than a "Decoding frame" per each frame. It really shadows valid
information in the log during decoding.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] all: Don't set AVClass.item_name to its default value

[Kacper Michajlow]

On Mon, 25 Dec 2023 at 11:48, Zhao Zhili  wrote:
>
>
>
> > On Dec 25, 2023, at 18:21, Anton Khirnov  wrote:
> >
> > Quoting Zhao Zhili (2023-12-25 10:27:59)
> >>
> >>
> >>> On Dec 25, 2023, at 16:38, Anton Khirnov  wrote:
> >>>
> >>> Quoting Kacper Michajlow (2023-12-24 11:41:52)
> >>>> On Fri, 22 Dec 2023 at 14:57, Anton Khirnov  wrote:
> >>>>>
> >>>>> Quoting Andreas Rheinhardt (2023-12-22 14:48:45)
> >>>>>> Avoids relocations.
> >>>>>>
> >>>>>> Signed-off-by: Andreas Rheinhardt 
> >>>>>> ---
> >>>>>
> >>>>> Maybe mention that it's not needed after
> >>>>> acf63d5350adeae551d412db699f8ca03f7e76b9.
> >>>>
> >>>> This is not the only user of this API, no?
> >>>>
> >>>> I have a question for my own curiosity. This is ABI (and API) breaking
> >>>> change,
> >>>
> >>> It is not. This item was not guaranteed to be set, which was actually
> >>> the reason I wrote the patch that this one refers to.

It has never been marked as optional and while one can argue that it
was not marked as required either this ambiguity would be interpreted
that item_name is available by most. Moreover this is further
solidified by the first-party tools that made this assumption for more
than a decade.

I don't want to argue about technicalities and semantics, or whether
it is (was) required field. My point is that it is breaking change
that affects 3rd-party code and 1st-party code too.

You fixed log.c, but the exact same issue can be found in ffprobe.c
today. See 
https://github.com/FFmpeg/FFmpeg/blob/4fee63b241e0dd254436bf38df8b0635b2b666d8/fftools/ffprobe.c#L371
(and L385)

And by code search on github I could find more projects that are
affected. Some have directly copied previous version of the callback
from log.c.

> >> There is no problem to relax a restriction inside libavutil. However, 
> >> since there is
> >> no explicit documentation on whether item_name can be null or not, user 
> >> may make
> >> incorrect assumptions and depend on item_name not being null. I don’t 
> >> think break
> >> user’s code suddenly is a good idea, although we can say it’s break since 
> >> the beginning.
> >
> > My point is that the restriction never existed.
>
> I’m not argue against this.
>
> > There were always
> > AVClass instances that did not set that pointer. There were fewer of
> > them, but they did exist.
>
> acf63d5350adeae551d412db699f8ca03f7e76b9 just shows that even libavutil can 
> make
> that incorrect assumption, we cannot expect more from users. If user’s code 
> don’t trigger
> those instances with null pointer, it works from user’s viewpoint. It’s hard 
> to believe that
> the bug found in MPV doesn’t exist in other  projects. We need another method 
> to make
> such change smoothly.

Thanks, I couldn't summarize this better.

> >
> > --
> > Anton Khirnov
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org <mailto: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] all: Don't set AVClass.item_name to its default value

[Kacper Michajlow]

On Fri, 22 Dec 2023 at 14:57, Anton Khirnov  wrote:
>
> Quoting Andreas Rheinhardt (2023-12-22 14:48:45)
> > Avoids relocations.
> >
> > Signed-off-by: Andreas Rheinhardt 
> > ---
>
> Maybe mention that it's not needed after
> acf63d5350adeae551d412db699f8ca03f7e76b9.

This is not the only user of this API, no?

I have a question for my own curiosity. This is ABI (and API) breaking
change, shouldn't there be a documentation change to indicate this
item_name function pointer can be null now? Or otherwise a way to
indicate that this should be updated in client applications?

It is easy fix, but it is kinda surprising to break over 14 year old
code like in mpv, see: https://github.com/mpv-player/mpv/pull/13154

- Kacper

> LGTM otherwise.
>
> --
> Anton Khirnov
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] avcodec/pngdec: read colorspace info when decoding with AVDISCARD_ALL

[Kacper Michajlow]

On Tue, 28 Feb 2023 at 20:46, Leo Izen  wrote:
>
> On 2/27/23 11:34, Leo Izen wrote:
> > On 2/21/23 17:35, Leo Izen wrote:
> >> These chunks are lightweight and it's useful information to have when
> >> running ffmpeg -i or ffprobe, for example.
> >> ---
> >>   libavcodec/pngdec.c  | 136 ++-
> >>   tests/ref/fate/png-icc   |   8 +--
> >>   tests/ref/fate/png-side-data |   2 +-
> >>   3 files changed, 91 insertions(+), 55 deletions(-)
> >>
> >> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> >
> > I will push this soon-ish if there's no other objections.
> >
> > - Leo Izen (thebombzen)
> >
> >
>
> Pushed as fadfa147f812a3fe9e68723347d37b9cccd6222d.

Hi, Sorry for digging this old patch, but I'm curious.

What does "unsupported tv-range cICP chunk\n" and "we only support
pc-range RGB" mean?

Before this patch the range was properly set:
avctx->color_range = f->color_range =
s->cicp_range == 0 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG;

Now it is ignored with a warning. Neither commit message, nor the
comments in the code explain why the cICP range is now ignored in the
decoder and the frames are no longer tagged with proper color range.

As I understand the goal of the commit was to expose colorspace info
when AVDISCARD_ALL. Was cICP range change intentional?

Thanks,
Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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/amfenc: Fix for windows imprecise sleep

[Kacper Michajlow]

On Tue, 17 Oct 2023 at 19:34, Evgeny Pavlov  wrote:
>
> The reason for using av_usleep() here is that AMF API doesn’t provide an
> API for explicit wait. There are two modes to get output from encoder:
>
> 1. Polling with some sleep to avoid CPU thrashing – currently used in FFmpeg
>
> 2. Set timeout parameter on AMF encoder and QueryOutput call will block
> till output is available or the timeout happens.
>
> #2 is the preferable way but it is designed more to be used with a separate
> polling thread. With a single-thread approach in FFmpeg, the use of timeout
> can block input submission making things slower.  This is even more
> pronounced when B-frames are enabled and several inputs are needed to produce
> the first output.
>
> The condition of this sleep is in special events (primarily when amf input
> queue is full), not the core loop part. During the experiments the cpu
> increasing is about 2-4% or so, not a burst.
>
> For low resolution encoding,  these changes bring significant performance
> improvement (about 15%). It will not bring improvement for high resolution
> such as 4K.
>
>
> Thanks,
>
> Evgeny
>
> вт, 17 окт. 2023 г. в 03:26, Zhao Zhili :
>
> >
> > > 在 2023年10月17日，上午5:24，Mark Thompson  写道：
> > >
> > > On 16/10/2023 10:13, Evgeny Pavlov wrote:
> > >> This commit reduces the sleep time on Windows to improve AMF encoding
> > >> performance on low resolution input videos.
> > >> This fix is for Windows only, because sleep() function isn't
> > >> very accurate on Windows OS.
> > >> Fix for issue #10622
> > >> Signed-off-by: Evgeny Pavlov 
> > >> ---
> > >>  libavcodec/amfenc.c | 4 
> > >>  1 file changed, 4 insertions(+)
> > >> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> > >> index 061859f85c..0c95465d6e 100644
> > >> --- a/libavcodec/amfenc.c
> > >> +++ b/libavcodec/amfenc.c
> > >> @@ -770,7 +770,11 @@ int ff_amf_receive_packet(AVCodecContext *avctx,
> > AVPacket *avpkt)
> > >>  if (query_output_data_flag == 0) {
> > >>  if (res_resubmit == AMF_INPUT_FULL || ctx->delayed_drain
> > || (ctx->eof && res_query != AMF_EOF) || (ctx->hwsurfaces_in_queue >=
> > ctx->hwsurfaces_in_queue_max)) {
> > >>  block_and_wait = 1;
> > >> +#ifdef _WIN32
> > >> +av_usleep(0); //Sleep() is not precise on Windows OS.
> > >> +#else
> > >>  av_usleep(1000);
> > >> +#endif
> > >>  }
> > >>  }
> > >>  } while (block_and_wait);
> > >
> > > Wasting lots of power by spinning on a CPU core does not seem like a
> > good answer to this problem.  (I mean, presumably that is why Windows isn't
> > honouring your request for a short sleep, because it wants timers to have
> > larger gaps to avoid wasting power.)
> >
> > If av_usleep is implemented via Sleep like current case, sleep 0 means
> > yield current thread, so it’s not busy wait in normal case (but it can be
> > busy wait).
> >
> > av_usleep(500) may looks better and do the same job by depending 500/1000
> > = 0.
> >
> > I agree use sleep without real async is like a bug.
> >
> > >
> > > Why is there a sleep here at all, anyway?  An API for hardware encoding
> > should be providing a way for the caller to wait for an outstanding
> > operation to complete.
> > >
> > > Thanks,
> > >
> > > - Mark
> > > ___
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@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".

Please don't top-post. I'll bottom-post now and no one will know how
to read this email.

If you need more precise sleep on Windows, your application should use
timeBeginPeriod/timeEndPeriod API, see
https://learn.microsoft.com/en-us/windows/win32/api/timeapi/nf-timeapi-timebeginperiod

This sleep shouldn't be there to begin with and removing it only for
Windows, seems like a hacky workaround.

Sleep on Windows is accurate, when you request a timer resolution
appropriate for your application. You probably don't do that, and have
unexpectedly long sleeps, but it is not because they are "not
accurate", it is because you don't ask for it.

Side note, with `Sleep()` you can request only 1 ms sleep, but with
with waitable timers
https://learn.microsoft.com/en-us/windows/win32/sync/waitable-timer-objects
you can go down to 0.5 ms, which seems currently be the lowest
interval that Windows kernel will wake anything 

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Fri, 8 Sept 2023 at 20:46, Kieran Kunhya  wrote:
>
> On Fri, 8 Sept 2023 at 19:42, Kacper Michajlow  wrote:
>
> > On Fri, 8 Sept 2023 at 00:11, Kieran Kunhya  wrote:
> > >
> > > On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow 
> > wrote:
> > >
> > > > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis
> > > >  wrote:
> > > > >
> > > > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote:
> > > > > > What would be a downside of preferring CXX always if it exists?
> > > > >
> > > > > FFmpeg runs in a multitude of environments with a multitude of
> > > > portability
> > > > > requirements. Needlessly linking a C++ runtime is not OK.
> > > >
> > > > This does not answer my question. Let me rephrase. Do we know the case
> > > > where using C++ compiler driver rather than C would degrade the
> > > > quality of the resulting build?
> > > >
> > >
> > > The machine that ffmpeg is being compiled on is not necessarily the one
> > > that ffmpeg is going to be run on.
> >
> > Not sure how this is relevant to the discussion?
> >
>
> > > > > What would be a downside of preferring CXX always if it exists?
>
> Because it forces a dependency on libstdc++ even if another machine does
> not have it.

On all "affected" platforms, configure already adds -Wl,--as-needed,
so the resulting library has exactly the same dependencies. Unless
something references symbols from stdlib, but then it would just not
link with clang anyway.

~ clang++ m.o -Wl,--as-needed && ldd a.out
linux-vdso.so.1 (0x7ffeeb912000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f64050a7000)
/lib64/ld-linux-x86-64.so.2 (0x7f64052ad000)

~ clang m.o -Wl,--as-needed && ldd a.out
linux-vdso.so.1 (0x7ffe1fb36000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f5e7a3c2000)
/lib64/ld-linux-x86-64.so.2 (0x7f5e7a5c8000)

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Fri, 8 Sept 2023 at 01:35, Timo Rothenpieler via ffmpeg-devel
 wrote:
>
> On 07.09.2023 23:38, Kacper Michajlow wrote:
> > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis
> >  wrote:
> >>
> >> On 9/6/2023 6:31 PM, Kacper Michajlow wrote:
> >>> What would be a downside of preferring CXX always if it exists?
> >>
> >> FFmpeg runs in a multitude of environments with a multitude of portability
> >> requirements. Needlessly linking a C++ runtime is not OK.
> >
> > This does not answer my question. Let me rephrase. Do we know the case
> > where using C++ compiler driver rather than C would degrade the
> > quality of the resulting build?
> >
> > Using C++ driver would indeed append the (correct) runtime library to
> > the linker command, but if nothing references any symbols from it it
> > would not be linked. It is also why the current way of forcing
> > `lstdc++` kinda works, because it is silently ignored when not needed.
> >
> > Implementing logic to use C++ only when necessary is possible, but I'm
> > not a big fan of such automation. And in practice not sure how well it
> > would work, because it would require trying to link twice every
> > dependency in configure.
> >
> > Also the fact that "FFmpeg runs in a multitude of environment" is
> > precisely why I really don't like the current unconditional including
> > `-lstdc++`.
>
> Couldn't you just check if stdc++ is in the ldflags/extralibs, and if
> so, remove it, and use g++ to link?

Well, I'm lost now. Are you suggesting building on top of existing
hacks is a better solution than the proposed patch?

I refuse supporting any kind of random `-lstdc++` adding in configure
and then removing it in the end.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Fri, 8 Sept 2023 at 00:11, Kieran Kunhya  wrote:
>
> On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow  wrote:
>
> > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis
> >  wrote:
> > >
> > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote:
> > > > What would be a downside of preferring CXX always if it exists?
> > >
> > > FFmpeg runs in a multitude of environments with a multitude of
> > portability
> > > requirements. Needlessly linking a C++ runtime is not OK.
> >
> > This does not answer my question. Let me rephrase. Do we know the case
> > where using C++ compiler driver rather than C would degrade the
> > quality of the resulting build?
> >
>
> The machine that ffmpeg is being compiled on is not necessarily the one
> that ffmpeg is going to be run on.

Not sure how this is relevant to the discussion?

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis
 wrote:
>
> On 9/6/2023 6:31 PM, Kacper Michajlow wrote:
> > What would be a downside of preferring CXX always if it exists?
>
> FFmpeg runs in a multitude of environments with a multitude of portability
> requirements. Needlessly linking a C++ runtime is not OK.

This does not answer my question. Let me rephrase. Do we know the case
where using C++ compiler driver rather than C would degrade the
quality of the resulting build?

Using C++ driver would indeed append the (correct) runtime library to
the linker command, but if nothing references any symbols from it it
would not be linked. It is also why the current way of forcing
`lstdc++` kinda works, because it is silently ignored when not needed.

Implementing logic to use C++ only when necessary is possible, but I'm
not a big fan of such automation. And in practice not sure how well it
would work, because it would require trying to link twice every
dependency in configure.

Also the fact that "FFmpeg runs in a multitude of environment" is
precisely why I really don't like the current unconditional including
`-lstdc++`.

> Further, we do not generally automatically change build output based on
> what is available on the system.

I must kindly disagree. configure has 35 components marked as
`[autodetect]` and they are certainly changing the build output based
on what's available on the system. Whether it is intended or
historical behaviour is another question. But I'm also not proposing
to auto detect anything, quite the opposite with this silly manual
list of libraries.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Wed, 6 Sept 2023 at 19:09, Timo Rothenpieler  wrote:
>
> On 06.09.2023 18:54, Kacper Michajlow wrote:
> > On Wed, 6 Sept 2023 at 12:15, Timo Rothenpieler  
> > wrote:
> >>
> >> On 06/09/2023 01:26, Kacper Michajłow wrote:
> >>> Other C++ standard libraries exist. Also, this is not a proper way to
> >>> link the standard library anyway. Instead when a C++ dependency is
> >>> detected, switch to the C++ compiler driver to properly link everything.
> >>>
> >>> Signed-off-by: Kacper Michajłow 
> >>> ---
> >>>configure | 26 ++
> >>>1 file changed, 18 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/configure b/configure
> >>> index bd7f7697c8..f3ff48586a 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h 
> >>> machine_ioctl_bt848_h dev_video_bktr
> >>>caca_outdev_deps="libcaca"
> >>>decklink_deps_any="libdl LoadLibrary"
> >>>decklink_indev_deps="decklink threads"
> >>> -decklink_indev_extralibs="-lstdc++"
> >>>decklink_indev_suggest="libzvbi"
> >>>decklink_outdev_deps="decklink threads"
> >>>decklink_outdev_suggest="libklvanc"
> >>> -decklink_outdev_extralibs="-lstdc++"
> >>>dshow_indev_deps="IBaseFilter"
> >>>dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 
> >>> -lshlwapi"
> >>>fbdev_indev_deps="linux_fb_h"
> >>> @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC
> >>>test -n "$cc_type" && enable $cc_type ||
> >>>warn "Unknown C compiler $cc, unable to select optimal CFLAGS"
> >>>
> >>> +cxx_deps=(
> >>> +decklink
> >>> +libglslang
> >>> +libgme
> >>> +libopenmpt
> >>> +librubberband
> >>> +libsnappy
> >>> +)
> >>
> >> Can't say I'm a fan of maintaining a random list in the middle of
> >> configure, which manually contains all libs we think need libstdc++.
> >>
> >> This could for one change any time. But also, those dependenies might
> >> have other dependencies, which when linking statically might need 
> >> libstdc++.
> >>
> >> But in general, this seems extremely brittle and a maintenance nightmare.
> >
> > Yes, that's the main problem. Linking static dependencies is an
> > unsolved problem. We can discuss that, but wouldn't you agree that the
> > proposed patch is an improvement over the current status quo? Forcing
> > `-lstdc++` unconditionally, when some of the dependencies are enabled
> > is way worse. The logic of maintaining the list does not change, it is
> > just centralized in one place and properly switches linking language
> > to C++ when needed, instead side-loading stdc++.
>
> It's not really unsolved. The dependencies need to properly advertise
> their dependencies via pkg-config.
> That only becomes an issue if we have some legacy manual detection that
> bypass pkg-config by default, or weird projects that don't supply
> pkg-config files.

No, not really. I've explained why we shouldn't list standard
libraries in pkg-config. No need to repeat myself, feel free to refer
to my previous email.

And in fact I found the exact same discussion on pkg-config ML from a
few years ago.
https://lists.freedesktop.org/archives/pkg-config/2016-August/001053.html

The conclusion mostly matches mine argument, that we should not
include `-lstdc++` in `Libs.private`. Or any other implicitly added
standard lib for that matter.

> > Good solution that would not require any maintenance is to switch to
> > CXX always for linking.
>
> Just test if linking with CC/LD fails, and if it does, test if it works
> with CXX, and if so, use CXX?
> A bit crude, but at least does not cause a maintenance nightmare.

What would be a downside of preferring CXX always if it exists?

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Wed, 6 Sept 2023 at 12:15, Timo Rothenpieler  wrote:
>
> On 06/09/2023 01:26, Kacper Michajłow wrote:
> > Other C++ standard libraries exist. Also, this is not a proper way to
> > link the standard library anyway. Instead when a C++ dependency is
> > detected, switch to the C++ compiler driver to properly link everything.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> >   configure | 26 ++
> >   1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/configure b/configure
> > index bd7f7697c8..f3ff48586a 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h 
> > machine_ioctl_bt848_h dev_video_bktr
> >   caca_outdev_deps="libcaca"
> >   decklink_deps_any="libdl LoadLibrary"
> >   decklink_indev_deps="decklink threads"
> > -decklink_indev_extralibs="-lstdc++"
> >   decklink_indev_suggest="libzvbi"
> >   decklink_outdev_deps="decklink threads"
> >   decklink_outdev_suggest="libklvanc"
> > -decklink_outdev_extralibs="-lstdc++"
> >   dshow_indev_deps="IBaseFilter"
> >   dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 
> > -lshlwapi"
> >   fbdev_indev_deps="linux_fb_h"
> > @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC
> >   test -n "$cc_type" && enable $cc_type ||
> >   warn "Unknown C compiler $cc, unable to select optimal CFLAGS"
> >
> > +cxx_deps=(
> > +decklink
> > +libglslang
> > +libgme
> > +libopenmpt
> > +librubberband
> > +libsnappy
> > +)
>
> Can't say I'm a fan of maintaining a random list in the middle of
> configure, which manually contains all libs we think need libstdc++.
>
> This could for one change any time. But also, those dependenies might
> have other dependencies, which when linking statically might need libstdc++.
>
> But in general, this seems extremely brittle and a maintenance nightmare.

Yes, that's the main problem. Linking static dependencies is an
unsolved problem. We can discuss that, but wouldn't you agree that the
proposed patch is an improvement over the current status quo? Forcing
`-lstdc++` unconditionally, when some of the dependencies are enabled
is way worse. The logic of maintaining the list does not change, it is
just centralized in one place and properly switches linking language
to C++ when needed, instead side-loading stdc++.

Good solution that would not require any maintenance is to switch to
CXX always for linking.

> > +for l in ${cxx_deps[@]}; do
> > +enabled $l && ld_default=$cxx
> > +done
> > +
> >   : ${as_default:=$cc}
> >   : ${objcc_default:=$cc}
> >   : ${dep_cc_default:=$cc}
> > @@ -6706,12 +6716,12 @@ enabled libfribidi&& require_pkg_config 
> > libfribidi fribidi fribidi.h fri
> >   enabled libharfbuzz   && require_pkg_config libharfbuzz harfbuzz hb.h 
> > hb_buffer_create
> >   enabled libglslang && { check_lib spirv_compiler 
> > glslang/Include/glslang_c_interface.h glslang_initialize_process \
> >   -lglslang -lMachineIndependent -lOSDependent 
> > -lHLSL -lOGLCompiler -lGenericCodeGen \
> > --lSPVRemapper -lSPIRV -lSPIRV-Tools-opt 
> > -lSPIRV-Tools -lpthread -lstdc++ -lm ||
> > +-lSPVRemapper -lSPIRV -lSPIRV-Tools-opt 
> > -lSPIRV-Tools -lpthread -lm ||
> >   require spirv_compiler 
> > glslang/Include/glslang_c_interface.h glslang_initialize_process \
> >   -lglslang -lOSDependent -lHLSL -lOGLCompiler \
> > --lSPVRemapper -lSPIRV -lSPIRV-Tools-opt 
> > -lSPIRV-Tools -lpthread -lstdc++ -lm; }
> > +-lSPVRemapper -lSPIRV -lSPIRV-Tools-opt 
> > -lSPIRV-Tools -lpthread -lm; }
> >   enabled libgme&& { check_pkg_config libgme libgme gme/gme.h 
> > gme_new_emu ||
> > -   require libgme gme/gme.h gme_new_emu -lgme 
> > -lstdc++; }
> > +   require libgme gme/gme.h gme_new_emu -lgme; 
> > }
> >   enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
> >  check_lib libgsm "${gsm_hdr}" 
> > gsm_create -lgsm && break;
> >  done || die "ERROR: libgsm not found"; }
> > @@ -6770,7 +6780,7 @@ enabled libopencv && { check_headers 
> > opencv2/core/core_c.h &&
> >   enabled libopenh264   && require_pkg_config libopenh264 openh264 
> > wels/codec_api.h WelsGetCodecVersion
> >   enabled libopenjpeg   && { check_pkg_config libopenjpeg "libopenjp2 
> > >= 2.1.0" openjpeg.h opj_version ||
> >  { require_pkg_config libopenjpeg 
> > "libopenjp2 >= 2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags 
> > -DOPJ_STATIC; } }
> > -enabled libopenmpt&& require_pkg_config libopenmpt "libopenmpt >= 
> > 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append 
> > libopenmpt_extralibs "-lstdc++"
> 

Re: [FFmpeg-devel] [PATCH 1/2] configure: don't force specific C++ standard library linking

[Kacper Michajlow]

On Wed, 6 Sept 2023 at 00:17, Hendrik Leppkes  wrote:
>
> On Wed, Sep 6, 2023 at 12:14 AM Kacper Michajłow  wrote:
> >
> > Other C++ standard libraries exist. Also, this is not a proper way to
> > link the standard library anyway. Instead when a C++ dependency is
> > detected, switch to the C++ compiler driver to properly link everything.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> >  configure | 27 ++-
> >  1 file changed, 18 insertions(+), 9 deletions(-)
> >
> > diff --git a/configure b/configure
> > index bd7f7697c8..90ee6e4f7d 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h 
> > machine_ioctl_bt848_h dev_video_bktr
> >  caca_outdev_deps="libcaca"
> >  decklink_deps_any="libdl LoadLibrary"
> >  decklink_indev_deps="decklink threads"
> > -decklink_indev_extralibs="-lstdc++"
> >  decklink_indev_suggest="libzvbi"
> >  decklink_outdev_deps="decklink threads"
> >  decklink_outdev_suggest="libklvanc"
> > -decklink_outdev_extralibs="-lstdc++"
> >  dshow_indev_deps="IBaseFilter"
> >  dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 
> > -lshlwapi"
> >  fbdev_indev_deps="linux_fb_h"
> > @@ -4684,7 +4682,6 @@ msvc_common_flags(){
> >  -march=*) ;;
> >  -lz)  echo zlib.lib ;;
> >  -lx264)   echo libx264.lib ;;
> > --lstdc++) ;;
> >  -l*)  echo ${flag#-l}.lib ;;
> >  -LARGEADDRESSAWARE)   echo $flag ;;
> >  -L*)  echo -libpath:${flag#-L} ;;
>
> Should probably keep this one, it may come from a pkg-config file or
> some other source, and you don't want it forwarded to cl.exe
>
> - Hendrik

Fair point. Doesn't harm to keep it I guess. Although on Windows a lot
of unsupported things may come from .pc files. In general pkg-config
on Windows is tricky.

I've sent a new version of the patch.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] vulkan: enable VK_KHR_cooperative_matrix

[Kacper Michajlow]

On Sat, 26 Aug 2023 at 23:19, Lynne  wrote:
>
> Aug 13, 2023, 00:15 by kaspe...@gmail.com:
>
> > On Sat, 12 Aug 2023 at 16:45, Lynne  wrote:
> >
> >>
> >> It's of interest to API users, and of interest to us,
> >> as a DCT/DST can be implemented via matrix multiplies.
> >>
> >> Bumps up the required header version to 1.3.255, released
> >> 2 months ago, so it's had time to propagate.
> >>
> >
> > I would disagree, considering latest Vulkan SDK release is 1.3.250.1.
> > Please wait untill new version is released.
> >
>
> New SDK version 1.3.261.0 was released 5 days ago by LunarG,
> so I think everyone's had time to update their headers.
> Thanks.

Fine with me.

-Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] vulkan: enable VK_KHR_cooperative_matrix

[Kacper Michajlow]

On Sat, 12 Aug 2023 at 16:45, Lynne  wrote:
>
> It's of interest to API users, and of interest to us,
> as a DCT/DST can be implemented via matrix multiplies.
>
> Bumps up the required header version to 1.3.255, released
> 2 months ago, so it's had time to propagate.

I would disagree, considering latest Vulkan SDK release is 1.3.250.1.
Please wait untill new version is released.

> Patch attached.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@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 v2] avformat/http: copy only mime type from Content-Type

[Kacper Michajlow]

On Thu, 1 Jun 2023 at 21:44, Kacper Michajłow  wrote:
>
> Content-Type can include charset and boundary which is not a part of
> mime type and shouldn't be copied as such.
>
> Fixes HLS playback when the Content-Type includes additional fields.
>
> Signed-off-by: Kacper Michajłow 
> ---
>  libavformat/http.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 0817aafb5b..fd931c2d8e 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1205,7 +1205,7 @@ static int process_line(URLContext *h, char *line, int 
> line_count)
>  }
>  } else if (!av_strcasecmp(tag, "Content-Type")) {
>  av_free(s->mime_type);
> -s->mime_type = av_strdup(p);
> +s->mime_type = av_get_token((const char **), ";");
>  } else if (!av_strcasecmp(tag, "Set-Cookie")) {
>  if (parse_cookie(s, p, >cookie_dict))
>  av_log(h, AV_LOG_WARNING, "Unable to parse '%s'\n", p);
> --
> 2.34.1
>

Bump. I would prefer this smal thing to be fixed upstream, than adding
workaround.

Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/hls: Try to implement RFC8216 playlist refusal

[Kacper Michajlow]

On Mon, 15 May 2023 at 02:06, Michael Niedermayer
 wrote:
>
> This is not well tested and can likely be improved, just a
> hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1
>
> Should fix Ticket10353 (please test and report cases that still fail)
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/hls.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index df2442c376..790ae7a96a 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p)
>  strstr(p->buf, "#EXT-X-TARGETDURATION:") ||
>  strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) {
>
> -if (!av_match_ext(p->filename, "m3u8,hls,m3u")) {
> +int mime_ok = p->mime_type && !(
> +av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") &&
> +av_strcasecmp(p->mime_type, "audio/mpegurl") &&
> +av_strcasecmp(p->mime_type, "audio/x-mpegurl") &&
> +av_strcasecmp(p->mime_type, "application/x-mpegurl")

How about we AV_LOG_WARNING when non-standard/deprecated mime type is
used? If we want to be strict about rfc8216 only two first should be
supported. Of course for compatibility reasons likely we need to
support all of them, but warn about it would be nice touch.

> +);
> +
> +if (!av_match_ext(p->filename, "m3u8,hls,m3u") &&
> + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 &&

Where '.hls' came from? I don't think those are in fact used in the
wild? Maybe we can be strict and use only "m3u8,m3u"?

> +!mime_ok) {
>  av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non 
> standard extension\n");

This log wording is little bit off now, when there is no extension and
only mime matching.

>  return 0;
>  }
> --
> 2.17.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Minor remarks, but funcionally the patch resolves the issue. Thanks.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] Revert "avformat/hls: fail on probing non hls/m3u8 file extensions"

[Kacper Michajlow]

On Mon, 15 May 2023 at 01:06, Michael Niedermayer
 wrote:
>
> On Mon, May 15, 2023 at 12:40:50AM +0200, Kacper Michajlow wrote:
> > On Sun, 14 May 2023 at 23:39, Michael Niedermayer
> >  wrote:
> > >
> > > On Sun, May 14, 2023 at 09:41:29PM +0200, Anton Khirnov wrote:
> > > > This reverts commit 6b1f68ccb04d791f0250e05687c346a99ff47ea1, which
> > > > broke many streams in the wild
> > > >
> > > > Fixes #10353.
> > >
> > > This change violates a SHOULD in rfc8216 4.  Playlists
> > >
> > >Each Playlist file MUST be identifiable either by the path component
> > >of its URI or by HTTP Content-Type.
> >
> > either/or
> >
> > >  In the first case, the path MUST
> > >end with either .m3u8 or .m3u.
> >
> > First case, path component with valid extention.
> >
> > >  In the second, the HTTP Content-Type
> > >MUST be "application/vnd.apple.mpegurl" or "audio/mpegurl".
> >
> > Second (OR) case for URI with Content-Type.
> >
> > So current FFmpeg HEAD violate RFC, as it should allow URI with 
> > Content-Type.
>
> yes, code is buggy, i didnt see teh report before today night,
> and was doing too much non ffmpeg crap last 2-3 days so didnt see it before
>
> everyone is upset, noone mailed me, noone posted a good fix, noone reverted
>
> ill post something before going to sleep today probably but i likely
> wont have the time to properly test it before bed
> (testcases with ffmpeg / ffplay btw makes it easier to test than with
>  links to random webpages)
>
> thx
>

All good, take your time. I just wanted to clarify that current
implementation is too strict/buggy.

I think the upset comes from the fact that a lot of users started
flooding issue trackers with the complaints that their favorite live
stream doesn't work. And while the underlying issue is quite small
regression, there were uncertainty at first what is going on and a lot
of unnecessary comments and remarks were made.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] Revert "avformat/hls: fail on probing non hls/m3u8 file extensions"

[Kacper Michajlow]

On Sun, 14 May 2023 at 23:39, Michael Niedermayer
 wrote:
>
> On Sun, May 14, 2023 at 09:41:29PM +0200, Anton Khirnov wrote:
> > This reverts commit 6b1f68ccb04d791f0250e05687c346a99ff47ea1, which
> > broke many streams in the wild
> >
> > Fixes #10353.
>
> This change violates a SHOULD in rfc8216 4.  Playlists
>
>Each Playlist file MUST be identifiable either by the path component
>of its URI or by HTTP Content-Type.

either/or

>  In the first case, the path MUST
>end with either .m3u8 or .m3u.

First case, path component with valid extention.

>  In the second, the HTTP Content-Type
>MUST be "application/vnd.apple.mpegurl" or "audio/mpegurl".

Second (OR) case for URI with Content-Type.

So current FFmpeg HEAD violate RFC, as it should allow URI with Content-Type.

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://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] lavu/vulkan: fix handle type for 32-bit targets

[Kacper Michajlow]

On Mon, 6 Mar 2023 at 10:00, Martin Storsjö  wrote:
>
> On Thu, 2 Mar 2023, Kacper Michajłow wrote:
>
> > Fixes compilation with clang which errors out on Wint-conversion.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> > libavutil/hwcontext_vulkan.c | 2 +-
> > libavutil/vulkan.h   | 4 
> > 2 files changed, 5 insertions(+), 1 deletion(-)
>
> Minor context; it's only recent Clang that treats this issue as an error
> by default - iirc it changed sometime last year, so possibly since Clang
> 15 or so.
>
> >
> > diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
> > index 2a9b5f4aac..b3eccea7af 100644
> > --- a/libavutil/hwcontext_vulkan.c
> > +++ b/libavutil/hwcontext_vulkan.c
> > @@ -1149,7 +1149,7 @@ static void free_exec_ctx(AVHWFramesContext *hwfc, 
> > VulkanExecCtx *cmd)
> >
> > av_freep(>queues);
> > av_freep(>bufs);
> > -cmd->pool = NULL;
> > +cmd->pool = VK_NULL_HANDLE;
> > }
>
> This LGTM - I had run into the same issue and tried to fix this issue
> differently, but this looks better to me.
>
> > static VkCommandBuffer get_buf_exec_ctx(AVHWFramesContext *hwfc, 
> > VulkanExecCtx *cmd)
> > diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
> > index d1ea1e24fb..90922c6cf3 100644
> > --- a/libavutil/vulkan.h
> > +++ b/libavutil/vulkan.h
> > @@ -122,7 +122,11 @@ typedef struct FFVulkanPipeline {
> > VkDescriptorSetLayout *desc_layout;
> > VkDescriptorPool   desc_pool;
> > VkDescriptorSet   *desc_set;
> > +#if VK_USE_64_BIT_PTR_DEFINES == 1
> > void **desc_staging;
> > +#else
> > +uint64_t  *desc_staging;
> > +#endif
> > VkDescriptorSetLayoutBinding **desc_binding;
> > VkDescriptorUpdateTemplate*desc_template;
> > int   *desc_set_initialized;
> > --
> > 2.34.1
>
> What issue does this fix? I don't run into any errors relating to this,
> when building with Clang for Windows on i386 or armv7. I think the fix is
> correct though, the vulkan data types are a bit hard to get a grip of.

Indeed, by default `libavfilter/vulkan.c` is not built, even is vulkan
is enabled.
You can trigger it, with `--enable-libplacebo` for example.

It fixes those:

In file included from F:/dev/ffmpeg/libavfilter/vulk
an.c:19:
F:/dev/ffmpeg/libavutil/vulkan.c:1173:70: error: incompatible integer
to pointer conversion assigning to 'void *' from
'VkDescriptorSetLayout' (aka 'unsigned long long') [-Wint-conversion]
pl->desc_staging[spawn_pipeline_layout.setLayoutCount++] = pl->desc_layout[i];

F:/dev/ffmpeg/libavutil/vulkan.c:1272:29: error: incompatible integer
to pointer conversion assigning to 'void *' from 'VkDescriptorSet'
(aka 'unsigned long long') [-Wint-conversion]
pl->desc_staging[i] = pl->desc_set[i*pl->qf->nb_queues + pl->qf->cur_queue];

(sorry for double message)

-Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] configure: Fix Microsoft tools detection

[Kacper Michajlow]

On Wed, 26 Jan 2022 at 15:00, Martin Storsjö  wrote:
>
> Hi,
>
> On Sat, 22 Jan 2022, Kacper Michajłow wrote:
>
> > LLVM tools print installation path upon execution. If one uses LLVM
> > tools bundled with Microsoft Visual Studio installation, they would be
> > incorrectly detected as Microsoft's ones.
> >
> > Signed-off-by: Kacper Michajłow 
> > ---
> > configure | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> While the patch description seems to make sense, I wanted to try it out to
> see the practical effect for myself, and I fail to observe any difference.
>
> Can you provide your exact configure command line you use, where it makes
> a difference? I tried with "--cc=clang-cl --ld=lld-link --toolchain=msvc"
> and that works just as fine before this patch.
>
> In particular, the commands that you adjust run "$_cc -nologo-" and grep
> for "Microsoft" in the output of that. When I run that with clang-cl, it
> doesn't print a string containing "Microsoft".
>
> // Martin

Hi,

Yes you are right. In case of CC it doesn't change anything. clang-cl
prints installation dir only with `-v`. The main thing this patch
fixes is `--ar=llvm-ar` where it is mistaken for lib.exe and used with
wrong parameters. While fixing this I figured to make CC check also
more strict, because at some point it could be a problem. Sync all of
them to have same style as one that was already there
> elif $_cc 2>&1 | grep -q 'Microsoft.*ARM.*Assembler'; then

just for consistency...

Also I noticed that latest MSVC (19.30.30709) complains about
> cl : Command line warning D9035 : option 'nologo-' has been deprecated and 
> will be removed in a future release

But it doesn't affect anything, even if it were to be removed. Since
banner is shown always by default, unless `-nologo`. Just a side note
:)

-Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/[vf_nnedi, vf_estdif]: Check interlaced flag on correct frame

[Kacper Michajlow]

On Tue, 4 May 2021 at 21:05, Kacper Michajłow  wrote:

> Fixes regression in vf_nnedi after
> 24dc6d386c6f7edb8f6945319f53a7f0b1642bb8 and vf_estdif while at it.
>
> Signed-off-by: Kacper Michajłow 
> ---
>  libavfilter/vf_estdif.c | 2 +-
>  libavfilter/vf_nnedi.c  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavfilter/vf_estdif.c b/libavfilter/vf_estdif.c
> index 9dda604d3c..900e28ce67 100644
> --- a/libavfilter/vf_estdif.c
> +++ b/libavfilter/vf_estdif.c
> @@ -500,7 +500,7 @@ static int config_input(AVFilterLink *inlink)
>  return 0;
>  }
>
> -if ((s->deint && !in->interlaced_frame) || ctx->is_disabled) {
> +if ((s->deint && !s->prev->interlaced_frame) || ctx->is_disabled) {
>  s->prev->pts *= 2;
>  ret = ff_filter_frame(ctx->outputs[0], s->prev);
>  s->prev = in;
> diff --git a/libavfilter/vf_nnedi.c b/libavfilter/vf_nnedi.c
> index 6096e88812..686091ac8d 100644
> --- a/libavfilter/vf_nnedi.c
> +++ b/libavfilter/vf_nnedi.c
> @@ -695,7 +695,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>  return 0;
>  }
>
> -if ((s->deint && !in->interlaced_frame) || ctx->is_disabled) {
> +if ((s->deint && !s->prev->interlaced_frame) || ctx->is_disabled) {
>  s->prev->pts *= 2;
>  ret = ff_filter_frame(ctx->outputs[0], s->prev);
>  s->prev = in;
> --
> 2.28.0.308.g675a4aaf3b
>
>
While fixing this reggression in vf_nnedi I also noticed that yadif checks
also surrounding frames, but this is only in yadif and likely all
deinterlace filers would need to be adjusted if we want to have same logic,
so I left it as is.It works fine for my test file.

if ((yadif->deint && !yadif->cur->interlaced_frame) ||
ctx->is_disabled ||
(yadif->deint && !yadif->prev->interlaced_frame &&
yadif->prev->repeat_pict) ||
(yadif->deint && !yadif->next->interlaced_frame &&
yadif->next->repeat_pict)
) {
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] configure: Fix DEF file post-processing with LTO enabled.

[Kacper Michajlow]

2017-09-08 2:52 GMT+02:00 Michael Niedermayer <mich...@niedermayer.cc>:

> On Wed, Sep 06, 2017 at 08:03:18PM +0200, Kacper Michajlow wrote:
> > 2017-08-22 21:26 GMT+02:00 Kacper Michajłow <kaspe...@gmail.com>:
> >
> > > With LTO enabled exported symbol entry looks like:
> > > av_audio_convert @3 DATA
> > >
> > > In order to maintain valid format we need to strip everything after @.
> > >
> > > This patch fixes linking libraries compiled with MinGW toolchain with
> LTO
> > > enabled.
> > >
> > > Signed-off-by: Kacper Michajłow <kaspe...@gmail.com>
> > > ---
> > >  configure | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > >
> > Bump after two weeks.
>
> in absence of anyone else applying this.
>
> can you provide a testcase / command line to reproduce the issue
> this fixes
> maybe if its easy to reproduce it will get a reply sooner
>
> thxs
>


Tested with latest nevcairiel's mingw build from files.1f0.de/mingw and
VS2017

Setup env for lib.exe and link.exe (adjust for your VS instalation):
"C:\Program Files (x86)\Microsoft Visual
Studio\2017\Community\Common7\Tools\VsDevCmd.bat"

configure (let's disable everything for this cruel example):
sh configure --enable-lto --enable-shared --disable-static
--disable-everything --disable-avdevice --disable-avcodec
--disable-swresample --disable-swscale --disable-postproc

compile:
make

And now try to link something like

ver.c:
int main()
{
return avfilter_version();
}

cl ver.c libavfilter\avfilter.lib -link

It will fail without the patch, because of trailing DATA in .def file
avfilter.lib was not correctly created by lib.exe and is not usable as it
doesn't have those symbols listed.

If you need real project to reproduce you can add --enable-lto to LAV
Filters configure options and built it. Without this patch it will fail.

Regards,
Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: Fix DEF file post-processing with LTO enabled.

[Kacper Michajlow]

2017-08-22 21:26 GMT+02:00 Kacper Michajłow :

> With LTO enabled exported symbol entry looks like:
> av_audio_convert @3 DATA
>
> In order to maintain valid format we need to strip everything after @.
>
> This patch fixes linking libraries compiled with MinGW toolchain with LTO
> enabled.
>
> Signed-off-by: Kacper Michajłow 
> ---
>  configure | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
>
Bump after two weeks.

Thanks,
Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: disable the new optimizer in Visual Studio 2015 Update 3

[Kacper Michajlow]

2016-07-04 3:53 GMT+02:00 Kacper Michajlow <kaspe...@gmail.com>:
> 2016-07-03 23:39 GMT+02:00 Hendrik Leppkes <h.lepp...@gmail.com>:
>> On Tue, Jun 28, 2016 at 12:01 PM, Hendrik Leppkes <h.lepp...@gmail.com> 
>> wrote:
>>> On Tue, Jun 28, 2016 at 11:48 AM, Hendrik Leppkes <h.lepp...@gmail.com> 
>>> wrote:
>>>> Visual Studio 2015 Update 3 introduced a new SSA optimizer, however
>>>> it unfortunately causes miscompilations. Until it is fixed, the new
>>>> optimizations are disabled and should be re-checked on subsequent
>>>> compiler releases.
>>>>
>>>> Fixes recent FATE failure of fate-lavf-pam on VS2015.
>>>
>>> On that note, i'm not exactly sure which code actually miscompiles.
>>> I though its pamenc, but the generated files are identical, then I
>>> though its the crc computing code (crcenc/adler32), but those seem
>>> fine as well and would likely screw up in more then one case if they
>>> would miscompile.
>>>
>>> So that leaves pnmdec, I suppose, but I didn't make much headway to
>>> poke around in that. If anyone has any handy hints how to find the
>>> code in question that might be helpful.
>>> Maybe the code is actually not-quite-right and might enjoy some 
>>> straigtening.
>>>
>>
>> Applied the patch to disable the new optimizations in MSVC as I did
>> not spot anything obviously "bad" in pamdec - which does not mean
>> there isn't anything there, of course.
>>
>
> This is pretty bad miscompilation in new SSA optimizer. It assumes
> that variable declared in loop doesn't change the value even if there
> is assignment. I minimized testcese and reported the bug
> https://connect.microsoft.com/VisualStudio/feedback/details/2890170
>
> - Kacper

Sorry for bumping such old thread, but I just got information that the
bug has been fixed in MSVC.

It is fixed in hotfix for VS2015 - KB3207317
https://support.microsoft.com/en-us/kb/3207317 and of course in
upcoming VS2017.

I've confirmed that this hotfix fixes fate-lavf-pam test. The
following change will enable SSA Optimizer on newer compiler versions.

---
 configure | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 398e843..cf82b3b 100755
--- a/configure
+++ b/configure
@@ -6317,9 +6317,9 @@ EOF
 check_func strtoll || add_cflags -Dstrtoll=_strtoi64
 check_func strtoull || add_cflags -Dstrtoull=_strtoui64
 # the new SSA optimzer in VS2015 U3 is mis-optimizing some parts
of the code
-# this flag should be re-checked on newer compiler releases and put under a
-# version check once its fixed
-check_cflags -d2SSAOptimizer-
+# Issue has been fixed in MSVC v19.00.24218.
+check_cpp_condition windows.h "_MSC_FULL_VER >= 190024218" ||
+check_cflags -d2SSAOptimizer-
 fi

 for pfx in "" host_; do
-- 
2.10.0.windows.1.325.ge6089c1

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: disable the new optimizer in Visual Studio 2015 Update 3

[Kacper Michajlow]

2016-07-03 23:39 GMT+02:00 Hendrik Leppkes :
> On Tue, Jun 28, 2016 at 12:01 PM, Hendrik Leppkes  wrote:
>> On Tue, Jun 28, 2016 at 11:48 AM, Hendrik Leppkes  
>> wrote:
>>> Visual Studio 2015 Update 3 introduced a new SSA optimizer, however
>>> it unfortunately causes miscompilations. Until it is fixed, the new
>>> optimizations are disabled and should be re-checked on subsequent
>>> compiler releases.
>>>
>>> Fixes recent FATE failure of fate-lavf-pam on VS2015.
>>
>> On that note, i'm not exactly sure which code actually miscompiles.
>> I though its pamenc, but the generated files are identical, then I
>> though its the crc computing code (crcenc/adler32), but those seem
>> fine as well and would likely screw up in more then one case if they
>> would miscompile.
>>
>> So that leaves pnmdec, I suppose, but I didn't make much headway to
>> poke around in that. If anyone has any handy hints how to find the
>> code in question that might be helpful.
>> Maybe the code is actually not-quite-right and might enjoy some straigtening.
>>
>
> Applied the patch to disable the new optimizations in MSVC as I did
> not spot anything obviously "bad" in pamdec - which does not mean
> there isn't anything there, of course.
>

This is pretty bad miscompilation in new SSA optimizer. It assumes
that variable declared in loop doesn't change the value even if there
is assignment. I minimized testcese and reported the bug
https://connect.microsoft.com/VisualStudio/feedback/details/2890170

- Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/nellymoserenc: avoid wasteful pow

[Kacper Michajlow]

One minor nitpick about commit message. You could mention which compiler
was used to generate code for benchmark. For example Clang 3.7 replaces
pow(2,...) with exp2(...) call by itself. So you probably did use gcc.
Anyway since it is already merged I guess take my reply as a hint for next
time :)

Regards,
Kacper
17 gru 2015 5:14 PM "Ganesh Ajjanagadde"  napisał(a):

> On Tue, Dec 15, 2015 at 6:40 PM, Ganesh Ajjanagadde 
> wrote:
> > On Tue, Dec 15, 2015 at 5:25 PM, Ganesh Ajjanagadde 
> wrote:
> >> On Tue, Dec 15, 2015 at 2:23 AM, Michael Niedermayer 
> wrote:
> >>> On Wed, Dec 09, 2015 at 06:55:25PM -0500, Ganesh Ajjanagadde wrote:
> > [...]
> 
>  diff --git a/libavcodec/nellymoserenc.c b/libavcodec/nellymoserenc.c
>  index d998dba..e6023e3 100644
>  --- a/libavcodec/nellymoserenc.c
>  +++ b/libavcodec/nellymoserenc.c
>  @@ -179,8 +179,15 @@ static av_cold int encode_init(AVCodecContext
> *avctx)
> 
>   /* Generate overlap window */
>   ff_init_ff_sine_windows(7);
>  -for (i = 0; i < POW_TABLE_SIZE; i++)
>  -pow_table[i] = pow(2, -i / 2048.0 - 3.0 + POW_TABLE_OFFSET);
>  +pow_table[0] = 1;
>  +pow_table[1024] = M_SQRT1_2;
>  +for (i = 1; i < 513; i++) {
>  +double tmp = exp2(-i / 2048.0);
>  +pow_table[i] = tmp;
>  +pow_table[1024-i] = M_SQRT1_2 / tmp;
>  +pow_table[1024+i] = tmp * M_SQRT1_2;
>  +pow_table[2048-i] = 0.5 / tmp;
> >>>
> >>> how much overall init time is gained by this ?
> >>> that is time in ffmpeg main() from start to finish when just opening
> >>> the file with no decoding aka ./ffmpeg -i somefile
> >>
> >> Don't know, all I know is cycles are unnecessarily wasted. Will put in
> >> cycle numbers.
> >>
> >
> > Here they are:
> > proposed: 424160 decicycles in pow_table, 512 runs,  0 skips
> > exp2 only: 1262093 decicycles in pow_table, 512 runs,  0 skips
> > old: 2849085 decicycles in pow_table, 512 runs,  0 skips
> >
> > Thus old to exp2 is roughly 2.25x speedup, exp2 to proposed roughly 3x
> > speedup, net ~ 6.7x speedup.
>
> took Michael's comment as a general ack, so pushed with addition of a
> comment and cycle numbers.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/nellymoserenc: avoid wasteful pow

[Kacper Michajlow]

18 gru 2015 10:06 AM "Kacper Michajlow" <kaspe...@gmail.com> napisał(a):
>
> One minor nitpick about commit message. You could mention which compiler
was used to generate code for benchmark. For example Clang 3.7 replaces
pow(2,...) with exp2(...) call by itself. So you probably did use gcc.
Anyway since it is already merged I guess take my reply as a hint for next
time :)
>
> Regards,
> Kacper
>
> 17 gru 2015 5:14 PM "Ganesh Ajjanagadde" <gajja...@mit.edu> napisał(a):
>>
>> On Tue, Dec 15, 2015 at 6:40 PM, Ganesh Ajjanagadde <gajja...@mit.edu>
wrote:
>> > On Tue, Dec 15, 2015 at 5:25 PM, Ganesh Ajjanagadde <gajja...@mit.edu>
wrote:
>> >> On Tue, Dec 15, 2015 at 2:23 AM, Michael Niedermayer <michae...@gmx.at>
wrote:
>> >>> On Wed, Dec 09, 2015 at 06:55:25PM -0500, Ganesh Ajjanagadde wrote:
>> > [...]
>> >>>>
>> >>>> diff --git a/libavcodec/nellymoserenc.c b/libavcodec/nellymoserenc.c
>> >>>> index d998dba..e6023e3 100644
>> >>>> --- a/libavcodec/nellymoserenc.c
>> >>>> +++ b/libavcodec/nellymoserenc.c
>> >>>> @@ -179,8 +179,15 @@ static av_cold int encode_init(AVCodecContext
*avctx)
>> >>>>
>> >>>>  /* Generate overlap window */
>> >>>>  ff_init_ff_sine_windows(7);
>> >>>> -for (i = 0; i < POW_TABLE_SIZE; i++)
>> >>>> -pow_table[i] = pow(2, -i / 2048.0 - 3.0 +
POW_TABLE_OFFSET);
>> >>>> +pow_table[0] = 1;
>> >>>> +pow_table[1024] = M_SQRT1_2;
>> >>>> +for (i = 1; i < 513; i++) {
>> >>>> +double tmp = exp2(-i / 2048.0);
>> >>>> +pow_table[i] = tmp;
>> >>>> +pow_table[1024-i] = M_SQRT1_2 / tmp;
>> >>>> +pow_table[1024+i] = tmp * M_SQRT1_2;
>> >>>> +pow_table[2048-i] = 0.5 / tmp;
>> >>>
>> >>> how much overall init time is gained by this ?
>> >>> that is time in ffmpeg main() from start to finish when just opening
>> >>> the file with no decoding aka ./ffmpeg -i somefile
>> >>
>> >> Don't know, all I know is cycles are unnecessarily wasted. Will put in
>> >> cycle numbers.
>> >>
>> >
>> > Here they are:
>> > proposed: 424160 decicycles in pow_table, 512 runs,  0 skips
>> > exp2 only: 1262093 decicycles in pow_table, 512 runs,  0 skips
>> > old: 2849085 decicycles in pow_table, 512 runs,  0 skips
>> >
>> > Thus old to exp2 is roughly 2.25x speedup, exp2 to proposed roughly 3x
>> > speedup, net ~ 6.7x speedup.
>>
>> took Michael's comment as a general ack, so pushed with addition of a
>> comment and cycle numbers.
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Sorry for top post.

-Kacper
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel