Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Wed, 23 May 2018 20:25:34 +0100 Rostislav Pehlivanov wrote: > On 23 May 2018 at 20:01, wm4 wrote: > > > On Wed, 23 May 2018 14:29:38 -0400 (EDT) > > Patrick Keroulas wrote: > > > > > - Original Message - > > > > From: "wm4" > > > > To: ffmpeg-devel@ffmpeg.org > > > > Sent: Wednesday, May 23, 2018 12:02:45 PM > > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for > > packets with top/bottom field > > > > > > > On Wed, 23 May 2018 16:46:17 +0100 > > > > Rostislav Pehlivanov wrote: > > > > > > > >> On 23 May 2018 at 16:18, wm4 wrote: > > > >> > > > >> > On Tue, 22 May 2018 17:19:35 -0400 (EDT) > > > >> > Patrick Keroulas wrote: > > > >> > > > > >> > > - Original Message - > > > >> > > > From: "Rostislav Pehlivanov" > > > >> > > > To: "FFmpeg development discussions and patches" < > > > >> > ffmpeg-devel@ffmpeg.org> > > > >> > > > Sent: Friday, May 18, 2018 5:28:42 PM > > > >> > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags > > for > > > >> > packets with top/bottom field > > > >> > > > > > >> > > > On 18 May 2018 at 22:17, wm4 wrote: > > > >> > > > > > >> > > > > > >> > > > > > >> > > > > But I think a new side data type would be much saner. We > > could even > > > >> > > > > just make it something generic, like AV_PKT_DATA_ANCILLARY or > > > >> > > > > something. It's apparently just packet data which somehow > > couldn't go > > > >> > > > > into the packet data. > > > >> > > > > > >> > > > > > >> > > > I agree, a generic ancillary side data type sounds better. It > > would > > > >> > have to > > > >> > > > be handled the same way as mastering metadata (e.g. to allocate > > it > > > >> > you'd > > > >> > > > need to use a separate function), since the size of the data > > struct > > > >> > can't > > > >> > > > be part of the API if we intend to add fields later. > > > >> > > > Patrick, if you're okay with that you should submit a patch > > which bases > > > >> > > > such side data on libavutil/mastering_display_metadata.h/.c > > > >> > > > > > >> > > No problem for transmitting field flags through side data. But > > the given > > > >> > > example (libavutil/mastering_display_metadata.h/.c) attaches > > data to > > > >> > > AVFrame, not AVPacket, so I'm not sure where to place this > > separate > > > >> > > allocator function. Do you recommend to create a new > > > >> > > libavcodec/ancillary.c/h utility? > > > >> > > > > >> > The example you mentioned exists for AVPacket too (it's just not > > easy > > > >> > to see how it can end up in AVPacket, because no demuxer does that > > > >> > directly). > > > >> > > > > >> > Anyway, ancillary side data would just be an untyped byte array, so > > I > > > >> > don't think it needs any helpers. Just an addition to the packet > > side > > > >> > data enum (I think it's somewhere in avcodec.h). > > > >> > ___ > > > >> > ffmpeg-devel mailing list > > > >> > ffmpeg-devel@ffmpeg.org > > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > >> > > > > >> > > > >> I'd rather have it as a well defined typed array rather than a bunch > > of > > > >> bytes. Otherwise we'd start sending unknown side data info and users > > > >> wouldn't know what to do with it. > > > > > > > > Unless you're adding some meta object system for describing arbitrary > > > > types at runtime I don't know how you'd do that. > &
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Wed, 23 May 2018 14:29:38 -0400 (EDT) Patrick Keroulas wrote: > - Original Message - > > From: "wm4" > > To: ffmpeg-devel@ffmpeg.org > > Sent: Wednesday, May 23, 2018 12:02:45 PM > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets > > with top/bottom field > > > On Wed, 23 May 2018 16:46:17 +0100 > > Rostislav Pehlivanov wrote: > > > >> On 23 May 2018 at 16:18, wm4 wrote: > >> > >> > On Tue, 22 May 2018 17:19:35 -0400 (EDT) > >> > Patrick Keroulas wrote: > >> > > >> > > - Original Message - > >> > > > From: "Rostislav Pehlivanov" > >> > > > To: "FFmpeg development discussions and patches" < > >> > ffmpeg-devel@ffmpeg.org> > >> > > > Sent: Friday, May 18, 2018 5:28:42 PM > >> > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for > >> > packets with top/bottom field > >> > > > >> > > > On 18 May 2018 at 22:17, wm4 wrote: > >> > > > >> > > > >> > > > >> > > > > But I think a new side data type would be much saner. We could even > >> > > > > just make it something generic, like AV_PKT_DATA_ANCILLARY or > >> > > > > something. It's apparently just packet data which somehow couldn't > >> > > > > go > >> > > > > into the packet data. > >> > > > >> > > > >> > > > I agree, a generic ancillary side data type sounds better. It would > >> > have to > >> > > > be handled the same way as mastering metadata (e.g. to allocate it > >> > you'd > >> > > > need to use a separate function), since the size of the data struct > >> > can't > >> > > > be part of the API if we intend to add fields later. > >> > > > Patrick, if you're okay with that you should submit a patch which > >> > > > bases > >> > > > such side data on libavutil/mastering_display_metadata.h/.c > >> > > > >> > > No problem for transmitting field flags through side data. But the > >> > > given > >> > > example (libavutil/mastering_display_metadata.h/.c) attaches data to > >> > > AVFrame, not AVPacket, so I'm not sure where to place this separate > >> > > allocator function. Do you recommend to create a new > >> > > libavcodec/ancillary.c/h utility? > >> > > >> > The example you mentioned exists for AVPacket too (it's just not easy > >> > to see how it can end up in AVPacket, because no demuxer does that > >> > directly). > >> > > >> > Anyway, ancillary side data would just be an untyped byte array, so I > >> > don't think it needs any helpers. Just an addition to the packet side > >> > data enum (I think it's somewhere in avcodec.h). > >> > ___ > >> > ffmpeg-devel mailing list > >> > ffmpeg-devel@ffmpeg.org > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > >> > >> I'd rather have it as a well defined typed array rather than a bunch of > >> bytes. Otherwise we'd start sending unknown side data info and users > >> wouldn't know what to do with it. > > > > Unless you're adding some meta object system for describing arbitrary > > types at runtime I don't know how you'd do that. > > Is that ok if I simply define a basic struct to hold the field? > > Any suggestion on where to insert the definition of this data and the > accessors in lavc? In a new source file? If you make it a struct, then make a new file in libavutil, with at least a helper to get the struct size (this is for ABI reasons, so we can extend the struct later). But then this side data would need a specific name, not a generic one like "ancillary". The display mastering thing is valid for both packets and frames, which might be confusing. The thing you add is needed for packets only. I'd prefer the "ancillary" name and making it just a flat byte array instead of a struct and something specific. The former would be like extradata, just per packet. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Wed, 23 May 2018 16:46:17 +0100 Rostislav Pehlivanov wrote: > On 23 May 2018 at 16:18, wm4 wrote: > > > On Tue, 22 May 2018 17:19:35 -0400 (EDT) > > Patrick Keroulas wrote: > > > > > - Original Message - > > > > From: "Rostislav Pehlivanov" > > > > To: "FFmpeg development discussions and patches" < > > ffmpeg-devel@ffmpeg.org> > > > > Sent: Friday, May 18, 2018 5:28:42 PM > > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for > > packets with top/bottom field > > > > > > > On 18 May 2018 at 22:17, wm4 wrote: > > > > > > > > > > > > > > But I think a new side data type would be much saner. We could even > > > > > just make it something generic, like AV_PKT_DATA_ANCILLARY or > > > > > something. It's apparently just packet data which somehow couldn't go > > > > > into the packet data. > > > > > > > > > > I agree, a generic ancillary side data type sounds better. It would > > have to > > > > be handled the same way as mastering metadata (e.g. to allocate it > > you'd > > > > need to use a separate function), since the size of the data struct > > can't > > > > be part of the API if we intend to add fields later. > > > > Patrick, if you're okay with that you should submit a patch which bases > > > > such side data on libavutil/mastering_display_metadata.h/.c > > > > > > No problem for transmitting field flags through side data. But the given > > > example (libavutil/mastering_display_metadata.h/.c) attaches data to > > > AVFrame, not AVPacket, so I'm not sure where to place this separate > > > allocator function. Do you recommend to create a new > > > libavcodec/ancillary.c/h utility? > > > > The example you mentioned exists for AVPacket too (it's just not easy > > to see how it can end up in AVPacket, because no demuxer does that > > directly). > > > > Anyway, ancillary side data would just be an untyped byte array, so I > > don't think it needs any helpers. Just an addition to the packet side > > data enum (I think it's somewhere in avcodec.h). > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > I'd rather have it as a well defined typed array rather than a bunch of > bytes. Otherwise we'd start sending unknown side data info and users > wouldn't know what to do with it. Unless you're adding some meta object system for describing arbitrary types at runtime I don't know how you'd do that. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Tue, 22 May 2018 17:19:35 -0400 (EDT) Patrick Keroulas wrote: > - Original Message - > > From: "Rostislav Pehlivanov" > > To: "FFmpeg development discussions and patches" > > Sent: Friday, May 18, 2018 5:28:42 PM > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets > > with top/bottom field > > > On 18 May 2018 at 22:17, wm4 wrote: > > > > > > But I think a new side data type would be much saner. We could even > > > just make it something generic, like AV_PKT_DATA_ANCILLARY or > > > something. It's apparently just packet data which somehow couldn't go > > > into the packet data. > > > > I agree, a generic ancillary side data type sounds better. It would have to > > be handled the same way as mastering metadata (e.g. to allocate it you'd > > need to use a separate function), since the size of the data struct can't > > be part of the API if we intend to add fields later. > > Patrick, if you're okay with that you should submit a patch which bases > > such side data on libavutil/mastering_display_metadata.h/.c > > No problem for transmitting field flags through side data. But the given > example (libavutil/mastering_display_metadata.h/.c) attaches data to > AVFrame, not AVPacket, so I'm not sure where to place this separate > allocator function. Do you recommend to create a new > libavcodec/ancillary.c/h utility? The example you mentioned exists for AVPacket too (it's just not easy to see how it can end up in AVPacket, because no demuxer does that directly). Anyway, ancillary side data would just be an untyped byte array, so I don't think it needs any helpers. Just an addition to the packet side data enum (I think it's somewhere in avcodec.h). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH][ticket #5522] lavc/cfhd: interlaced frame decoding added
On Wed, 23 May 2018 00:21:56 +0200
Hendrik Leppkes wrote:
> On Tue, May 22, 2018 at 10:35 PM, Carl Eugen Hoyos wrote:
> > 2018-05-22 17:40 GMT+02:00, Gagandeep Singh :
> >
> >> +low= s->plane[plane].subband[0];
> >> +high = s->plane[plane].subband[8];
> >> +output = s->plane[plane].l_h[6];
> >> +for (i = 0; i < lowpass_width; i++) {
> >> +vert_filter(output, lowpass_width, low, lowpass_width,
> >> high, highpass_stride, lowpass_height);
> >> +low++;
> >> +high++;
> >> +output++;
> >> +}
> >>
> >> -low= s->plane[plane].subband[0];
> >> -high = s->plane[plane].subband[8];
> >> -output = s->plane[plane].l_h[6];
> >> -for (i = 0; i < lowpass_width; i++) {
> >> -vert_filter(output, lowpass_width, low, lowpass_width, high,
> >> highpass_stride, lowpass_height);
> >> -low++;
> >> -high++;
> >> -output++;
> >> -}
> >
> > The patch will get much more readable (and easier to review)
> > if you do not re-indent, instead send a second patch with the
> > cosmetic changes only.
> >
>
> Except, no sane developer works like that. In fact many editors will
> even indent for you automatically if you add new blocks, so splitting
> this is a really annoying task to perform.
> Instead, may I suggest a proper patch viewer which can ignore
> whitespace changes? It makes reading patches with indent changes
> trivial.
+1
We could add instructions to the docs how to make patches with
whitespace changes ignored.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] lavu/hwcontext_vaapi: add nv21 map
On Wed, 23 May 2018 18:29:20 +0800
Jun Zhao wrote:
> Signed-off-by: Jun Zhao
> ---
> libavutil/hwcontext_vaapi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c
> index 5bdb02f..7b3cbea 100644
> --- a/libavutil/hwcontext_vaapi.c
> +++ b/libavutil/hwcontext_vaapi.c
> @@ -100,6 +100,9 @@ static const struct {
> enum AVPixelFormat pix_fmt;
> } vaapi_format_map[] = {
> MAP(NV12, YUV420, NV12),
> +#ifdef VA_FOURCC_NV21
> +MAP(NV21, YUV420, NV21),// As NV12 with U and V reversed
> +#endif
> MAP(YV12, YUV420, YUV420P), // With U/V planes swapped.
> MAP(IYUV, YUV420, YUV420P),
> #ifdef VA_FOURCC_I420
Why does this format exist and why is it "needed"?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Fri, 18 May 2018 21:59:13 +0100
Rostislav Pehlivanov wrote:
> On 18 May 2018 at 21:03, wm4 wrote:
>
> > On Fri, 18 May 2018 20:09:02 +0100
> > Rostislav Pehlivanov wrote:
> >
> > > On 18 May 2018 at 20:05, Patrick Keroulas <
> > > patrick.kerou...@savoirfairelinux.com> wrote:
> > >
> > > >
> > > > - Original Message -
> > > > > From: "Rostislav Pehlivanov"
> > > > > To: "FFmpeg development discussions and patches" <
> > > > ffmpeg-devel@ffmpeg.org>
> > > > > Sent: Tuesday, May 15, 2018 4:46:02 PM
> > > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for
> > > > packets with top/bottom field
> > > >
> > > > > On 15 May 2018 at 18:03, wm4 wrote:
> > > > >
> > > > >> On Tue, 15 May 2018 17:15:05 +0100
> > > > >> Rostislav Pehlivanov wrote:
> > > > >>
> > > > >> > On 15 May 2018 at 15:55, wm4 wrote:
> > > > >> >
> > > > >> > > On Mon, 14 May 2018 18:26:35 -0400
> > > > >> > > Patrick Keroulas wrote:
> > > > >> > >
> > > > >> > > > Signed-off-by: Patrick Keroulas > > > >> savoirfairelinux.com>
> > > > >> > > > ---
> > > > >> > > > doc/APIchanges | 3 +++
> > > > >> > > > libavcodec/avcodec.h | 8
> > > > >> > > > libavcodec/version.h | 4 ++--
> > > > >> > > > 3 files changed, 13 insertions(+), 2 deletions(-)
> > > > >> > > >
> > > > >> > > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > > >> > > > index bbefc83..d06868e 100644
> > > > >> > > > --- a/doc/APIchanges
> > > > >> > > > +++ b/doc/APIchanges
> > > > >> > > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> > > > >> > > >
> > > > >> > > > API changes, most recent first:
> > > > >> > > >
> > > > >> > > > +2018-05-xx - xx - lavc 58.20.100 - avcodec.h
> > > > >> > > > + Add AV_PKT_FLAG_TOP_FIELD and AV_PKT_FLAG_BOTTOM_FIELD.
> > > > >> > > > +
> > > > >> > > > 2018-05-xx - xx - lavu 56.18.101 - hwcontext_cuda.h
> > > > >> > > > Add AVCUDADeviceContext.stream.
> > > > >> > > >
> > > > >> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > > >> > > > index fb0c6fa..14811be 100644
> > > > >> > > > --- a/libavcodec/avcodec.h
> > > > >> > > > +++ b/libavcodec/avcodec.h
> > > > >> > > > @@ -1480,6 +1480,14 @@ typedef struct AVPacket {
> > > > >> > > > */
> > > > >> > > > #define AV_PKT_FLAG_DISPOSABLE 0x0010
> > > > >> > > >
> > > > >> > > > +/**
> > > > >> > > > + * The packet contains a top field.
> > > > >> > > > + */
> > > > >> > > > +#define AV_PKT_FLAG_TOP_FIELD 0x0020
> > > > >> > > > +/**
> > > > >> > > > + * The packet contains a bottom field.
> > > > >> > > > + */
> > > > >> > > > +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040
> > > > >> > > >
> > > > >> > > > enum AVSideDataParamChangeFlags {
> > > > >> > > > AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
> > > > >> > > > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > > > >> > > > index 3fda743..b9752ce 100644
> > > > >> > > > --- a/libavcodec/version.h
> > > > >> > > > +++ b/libavcodec/version.h
> > > > >> > > > @@ -28,8 +28,8 @@
> > > > >> > > > #include "libavutil/version.h"
> > > > >> > > >
> > > > >> > > > #define LIBAVCODEC_VERSION_MAJOR 58
> > > > >> > > > -#define LIBAVCODEC_VERSION_MINOR 19
> > > > >> > > > -#define L
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Fri, 18 May 2018 20:09:02 +0100
Rostislav Pehlivanov wrote:
> On 18 May 2018 at 20:05, Patrick Keroulas <
> patrick.kerou...@savoirfairelinux.com> wrote:
>
> >
> > - Original Message -
> > > From: "Rostislav Pehlivanov"
> > > To: "FFmpeg development discussions and patches" <
> > ffmpeg-devel@ffmpeg.org>
> > > Sent: Tuesday, May 15, 2018 4:46:02 PM
> > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for
> > packets with top/bottom field
> >
> > > On 15 May 2018 at 18:03, wm4 wrote:
> > >
> > >> On Tue, 15 May 2018 17:15:05 +0100
> > >> Rostislav Pehlivanov wrote:
> > >>
> > >> > On 15 May 2018 at 15:55, wm4 wrote:
> > >> >
> > >> > > On Mon, 14 May 2018 18:26:35 -0400
> > >> > > Patrick Keroulas wrote:
> > >> > >
> > >> > > > Signed-off-by: Patrick Keroulas > >> savoirfairelinux.com>
> > >> > > > ---
> > >> > > > doc/APIchanges | 3 +++
> > >> > > > libavcodec/avcodec.h | 8
> > >> > > > libavcodec/version.h | 4 ++--
> > >> > > > 3 files changed, 13 insertions(+), 2 deletions(-)
> > >> > > >
> > >> > > > diff --git a/doc/APIchanges b/doc/APIchanges
> > >> > > > index bbefc83..d06868e 100644
> > >> > > > --- a/doc/APIchanges
> > >> > > > +++ b/doc/APIchanges
> > >> > > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> > >> > > >
> > >> > > > API changes, most recent first:
> > >> > > >
> > >> > > > +2018-05-xx - xx - lavc 58.20.100 - avcodec.h
> > >> > > > + Add AV_PKT_FLAG_TOP_FIELD and AV_PKT_FLAG_BOTTOM_FIELD.
> > >> > > > +
> > >> > > > 2018-05-xx - xx - lavu 56.18.101 - hwcontext_cuda.h
> > >> > > > Add AVCUDADeviceContext.stream.
> > >> > > >
> > >> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > >> > > > index fb0c6fa..14811be 100644
> > >> > > > --- a/libavcodec/avcodec.h
> > >> > > > +++ b/libavcodec/avcodec.h
> > >> > > > @@ -1480,6 +1480,14 @@ typedef struct AVPacket {
> > >> > > > */
> > >> > > > #define AV_PKT_FLAG_DISPOSABLE 0x0010
> > >> > > >
> > >> > > > +/**
> > >> > > > + * The packet contains a top field.
> > >> > > > + */
> > >> > > > +#define AV_PKT_FLAG_TOP_FIELD 0x0020
> > >> > > > +/**
> > >> > > > + * The packet contains a bottom field.
> > >> > > > + */
> > >> > > > +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040
> > >> > > >
> > >> > > > enum AVSideDataParamChangeFlags {
> > >> > > > AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
> > >> > > > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > >> > > > index 3fda743..b9752ce 100644
> > >> > > > --- a/libavcodec/version.h
> > >> > > > +++ b/libavcodec/version.h
> > >> > > > @@ -28,8 +28,8 @@
> > >> > > > #include "libavutil/version.h"
> > >> > > >
> > >> > > > #define LIBAVCODEC_VERSION_MAJOR 58
> > >> > > > -#define LIBAVCODEC_VERSION_MINOR 19
> > >> > > > -#define LIBAVCODEC_VERSION_MICRO 101
> > >> > > > +#define LIBAVCODEC_VERSION_MINOR 20
> > >> > > > +#define LIBAVCODEC_VERSION_MICRO 100
> > >> > > >
> > >> > > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_
> > VERSION_MAJOR,
> > >>
> > >> > > \
> > >> > > >
> > >> > > LIBAVCODEC_VERSION_MINOR, \
> > >> > >
> > >> > > So far we could avoid codec-specific packet flags, and I think it
> > >> > > should stay this way. Maybe make it side data, something with naming
> > >> > > specific to the bitpacked codec. Or alternatively, if this codec
> > >> > > is 100% RTP specific and there's no such thing as standard bitpac
Re: [FFmpeg-devel] [PATCH] avformat/hls: Properly expose intercepted ID3 tags to the API.
On Fri, 18 May 2018 11:54:52 -0700
Richard Shaffer wrote:
> On Fri, May 18, 2018 at 2:54 AM, wm4 wrote:
>
> > On Thu, 17 May 2018 20:49:42 -0700
> > rshaf...@tunein.com wrote:
> >
> > > From: Richard Shaffer
> > >
> > > The HLS demuxer will process any ID3 tags at the beginning of a segment
> > in
> > > order to obtain timestamp data. However, when this data was parsed on a
> > second
> > > or subsequent segment, the updated metadata would be discarded. This
> > change
> > > preserves the data and also sets the AVSTREAM_EVENT_FLAG_METADATA_
> > UPDATED
> > > event flag when appropriate.
> > > ---
> > > libavformat/hls.c | 34 +++---
> > > 1 file changed, 19 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > > index 3d4f7f2647..3e2edb3484 100644
> > > --- a/libavformat/hls.c
> > > +++ b/libavformat/hls.c
> > > @@ -982,18 +982,8 @@ static void parse_id3(AVFormatContext *s,
> > AVIOContext *pb,
> > > }
> > >
> > > /* Check if the ID3 metadata contents have changed */
> > > -static int id3_has_changed_values(struct playlist *pls, AVDictionary
> > *metadata,
> > > - ID3v2ExtraMetaAPIC *apic)
> > > +static int id3_has_changed_values(struct playlist *pls,
> > ID3v2ExtraMetaAPIC *apic)
> > > {
> > > -AVDictionaryEntry *entry = NULL;
> > > -AVDictionaryEntry *oldentry;
> > > -/* check that no keys have changed values */
> > > -while ((entry = av_dict_get(metadata, "", entry,
> > AV_DICT_IGNORE_SUFFIX))) {
> > > -oldentry = av_dict_get(pls->id3_initial, entry->key, NULL,
> > AV_DICT_MATCH_CASE);
> > > -if (!oldentry || strcmp(oldentry->value, entry->value) != 0)
> > > -return 1;
> > > -}
> > > -
> > > /* check if apic appeared */
> > > if (apic && (pls->ctx->nb_streams != 2 || !pls->ctx->streams[1]->
> > attached_pic.data))
> > > return 1;
> > > @@ -1019,6 +1009,15 @@ static void handle_id3(AVIOContext *pb, struct
> > playlist *pls)
> > > int64_t timestamp = AV_NOPTS_VALUE;
> > >
> > > parse_id3(pls->ctx, pb, &metadata, ×tamp, &apic, &extra_meta);
> > > +ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
> > > +av_dict_copy(&pls->ctx->metadata, metadata, 0);
> > > +/*
> > > + * If we've handled any ID3 metadata here, it's not going to be
> > seen by the
> > > + * sub-demuxer. In the case that we got here because of an IO call
> > during
> > > + * hls_read_header, this will be cleared. Otherwise, it provides the
> > > + * necessary hint to hls_read_packet that there is new metadata.
> > > + */
> > > +pls->ctx->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
> >
> > Can't ID3 tags happen in large quantities with that ID3 timestamp hack
> > (curse whoever invented it)? Wouldn't that lead to a large number of
> > redundant events? Not sure though, I don't have the overview.
> >
>
> Yes, that's a good point. If timestamps are in ID3 tags, they'll be at the
> start of every segment. I can think of several ways to handle that:
>
> Option 1: Technically it is updated metadata, so mark it as updated. Leave
> it up to the API caller to figure out whether they care about it or not.
>
> Option 2: Filter out timestamp ID3 frames, and only mark it as updated if
> some other ID3 tag or frame is present.
>
> Option 3: Compare the new metadata with the last seen metadata, and only
> set the event flag if something besides the timestamp has actually changed.
> That would filter out false updates for the API consumer, but I'm pretty
> sure nothing else that handles metadata updates works this way.
>
> Any thoughts? Personally I'd lean towards 1 or 2.
2 sounds good.
>
> >
> > >
> > > if (timestamp != AV_NOPTS_VALUE) {
> > > pls->id3_mpegts_timestamp = timestamp;
> > > @@ -1037,12 +1036,10 @@ static void handle_id3(AVIOContext *pb, struct
> > playlist *pls)
> > > /* demuxer not yet opened, defer picture attachment */
> > > pls->id3_deferred_extra = extra_meta;
> > >
> > > -
Re: [FFmpeg-devel] [PATCH] avformat/mpegts: add skip_unknown_pmt option
On Thu, 17 May 2018 17:21:42 -0700 Aman Gupta wrote: > On Thu, May 17, 2018 at 5:04 PM, Aman Gupta wrote: > > > From: Aman Gupta > > > > Some filtered mpegts streams may erroneously include PMTs for programs > > that are not advertised in the PAT. This confuses ffmpeg and most > > players because multiple audio/video streams are created and it is > > unclear which ones actually contain data. > > > > See for example https://tmm1.s3.amazonaws.com/unknown-pmts.ts > > > > Before: > > > > Input #0, mpegts, from 'unknown-pmts.ts': > > Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s > > Program 4 > > Stream #0:2[0x41]: Video: mpeg2video (Main) ([2][0][0][0] / > > 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9], > > Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc > > Stream #0:3[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > > 5.1(side), fltp, 384 kb/s > > Stream #0:4[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > > stereo, fltp, 128 kb/s > > No Program > > Stream #0:0[0x31]: Video: mpeg2video ([2][0][0][0] / 0x0002), > > none(tv), 90k tbr, 90k tbn, 90k tbc > > Stream #0:1[0x34](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 > > channels, fltp > > Stream #0:5[0x51]: Video: mpeg2video ([2][0][0][0] / 0x0002), > > none, 90k tbr, 90k tbn > > Stream #0:6[0x54](eng): Audio: ac3 (AC-3 / 0x332D4341), 0 channels > > > > With skip_unknown_pmt=1: > > > > Input #0, mpegts, from 'unknown-pmts.ts': > > Duration: 00:00:10.11, start: 80741.189700, bitrate: 9655 kb/s > > Program 4 > > Stream #0:0[0x41]: Video: mpeg2video (Main) ([2][0][0][0] / > > 0x0002), yuv420p(tv, bt709, progressive), 1280x720 [SAR 1:1 DAR 16:9], > > Closed Captions, 11063 kb/s, 59.94 fps, 59.94 tbr, 90k tbn, 119.88 tbc > > Stream #0:1[0x44](eng): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > > 5.1(side), fltp, 384 kb/s > > Stream #0:2[0x45](spa): Audio: ac3 (AC-3 / 0x332D4341), 48000 Hz, > > stereo, fltp, 128 kb/s > > > > Signed-off-by: Aman Gupta > > --- > > doc/demuxers.texi| 3 +++ > > libavformat/mpegts.c | 5 + > > 2 files changed, 8 insertions(+) > > > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > > index e7c2abce57..1d2ee5bf37 100644 > > --- a/doc/demuxers.texi > > +++ b/doc/demuxers.texi > > @@ -538,6 +538,9 @@ This demuxer accepts the following options: > > Set size limit for looking up a new synchronization. Default value is > > 65536. > > > > +@item skip_unknown_pmt > > +Skip PMTs for programs not defined in the PAT. Default value is 0. > > > > Maybe it's worth turning this option on by default? Sounds reasonable, but I don't know too much about TS. > We could also add debug/verbose logging when these PMTs are skipped, > but since it would happen every time the PMT is received it could become > very noisy. Just make it a higher log level. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hls: Properly expose intercepted ID3 tags to the API.
On Thu, 17 May 2018 20:49:42 -0700
rshaf...@tunein.com wrote:
> From: Richard Shaffer
>
> The HLS demuxer will process any ID3 tags at the beginning of a segment in
> order to obtain timestamp data. However, when this data was parsed on a second
> or subsequent segment, the updated metadata would be discarded. This change
> preserves the data and also sets the AVSTREAM_EVENT_FLAG_METADATA_UPDATED
> event flag when appropriate.
> ---
> libavformat/hls.c | 34 +++---
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 3d4f7f2647..3e2edb3484 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -982,18 +982,8 @@ static void parse_id3(AVFormatContext *s, AVIOContext
> *pb,
> }
>
> /* Check if the ID3 metadata contents have changed */
> -static int id3_has_changed_values(struct playlist *pls, AVDictionary
> *metadata,
> - ID3v2ExtraMetaAPIC *apic)
> +static int id3_has_changed_values(struct playlist *pls, ID3v2ExtraMetaAPIC
> *apic)
> {
> -AVDictionaryEntry *entry = NULL;
> -AVDictionaryEntry *oldentry;
> -/* check that no keys have changed values */
> -while ((entry = av_dict_get(metadata, "", entry,
> AV_DICT_IGNORE_SUFFIX))) {
> -oldentry = av_dict_get(pls->id3_initial, entry->key, NULL,
> AV_DICT_MATCH_CASE);
> -if (!oldentry || strcmp(oldentry->value, entry->value) != 0)
> -return 1;
> -}
> -
> /* check if apic appeared */
> if (apic && (pls->ctx->nb_streams != 2 ||
> !pls->ctx->streams[1]->attached_pic.data))
> return 1;
> @@ -1019,6 +1009,15 @@ static void handle_id3(AVIOContext *pb, struct
> playlist *pls)
> int64_t timestamp = AV_NOPTS_VALUE;
>
> parse_id3(pls->ctx, pb, &metadata, ×tamp, &apic, &extra_meta);
> +ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
> +av_dict_copy(&pls->ctx->metadata, metadata, 0);
> +/*
> + * If we've handled any ID3 metadata here, it's not going to be seen by
> the
> + * sub-demuxer. In the case that we got here because of an IO call during
> + * hls_read_header, this will be cleared. Otherwise, it provides the
> + * necessary hint to hls_read_packet that there is new metadata.
> + */
> +pls->ctx->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
Can't ID3 tags happen in large quantities with that ID3 timestamp hack
(curse whoever invented it)? Wouldn't that lead to a large number of
redundant events? Not sure though, I don't have the overview.
>
> if (timestamp != AV_NOPTS_VALUE) {
> pls->id3_mpegts_timestamp = timestamp;
> @@ -1037,12 +1036,10 @@ static void handle_id3(AVIOContext *pb, struct
> playlist *pls)
> /* demuxer not yet opened, defer picture attachment */
> pls->id3_deferred_extra = extra_meta;
>
> -ff_id3v2_parse_priv_dict(&metadata, &extra_meta);
> -av_dict_copy(&pls->ctx->metadata, metadata, 0);
> pls->id3_initial = metadata;
>
> } else {
> -if (!pls->id3_changed && id3_has_changed_values(pls, metadata,
> apic)) {
> +if (!pls->id3_changed && id3_has_changed_values(pls, apic)) {
> avpriv_report_missing_feature(pls->ctx, "Changing ID3 metadata
> in HLS audio elementary stream");
> pls->id3_changed = 1;
> }
> @@ -1939,8 +1936,15 @@ static int hls_read_header(AVFormatContext *s)
> * Copy any metadata from playlist to main streams, but do not set
> * event flags.
> */
> -if (pls->n_main_streams)
> +if (pls->n_main_streams) {
> av_dict_copy(&pls->main_streams[0]->metadata,
> pls->ctx->metadata, 0);
> +/*
> + * If we've intercepted metadata, we will have set this event
> flag.
> + * Clear it to avoid confusion, since otherwise we will flag it
> as
> + * new metadata on the next call to hls_read_packet.
> + */
> +pls->ctx->event_flags = ~AVFMT_EVENT_FLAG_METADATA_UPDATED;
I think ~(unsigned)AVFMT_EVENT_... would be preferable for maximum
correctness.
> +}
>
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_AUDIO);
> add_metadata_from_renditions(s, pls, AVMEDIA_TYPE_VIDEO);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/mpegts: keep track of AVProgram.pmt_version and set AV_PROGRAM_CHANGED on version updates
On Thu, 17 May 2018 18:13:34 -0700 Aman Gupta wrote: > On Thu, May 17, 2018 at 3:49 PM, Aman Gupta wrote: > > > From: Aman Gupta > > > > This can be used to detect changes to the streams in an AVProgram > > > > Forgot to add: I have seen two related patches in the wild that attempt to > solve this same problem in different ways. > > The first is in MythTV's ffmpeg fork, where they added a void > (*streams_changed)(void*); to AVFormatContext and call it from their mpegts > demuxer when the PMT changes. > > The second was proposed by XBMC in > https://ffmpeg.org/pipermail/ffmpeg-devel/2012-December/135036.html, where > they created a new AVMEDIA_TYPE_DATA stream with id=0 and attempted to send > packets to it whenever the PMT changed. > > The approach in this patch is similar to what's used by > AVFormatContext.event_flags and AVFMT_EVENT_FLAG_METADATA_UPDATED. > > I re-used AVProgram.flags for this purpose (which appears not to be used > for anything else). To be more explicit, it might be better to add > AVProgram.event_flags. Note that either way, the user would need to clear > AV_PROGRAM_CHANGED after detecting it (which should be documented). > > Another possibility would be to remove AV_PROGRAM_CHANGED altogether, which > means the user would need to keep a copy of program->version and compare it > to detect changes. Probably needs new doxygen that says that the API user can clear that change flag when convenient. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/error: hidden macro av_err2str for C++
On Thu, 17 May 2018 05:50:55 +0100
Rostislav Pehlivanov wrote:
> On 17 May 2018 at 05:46, Zhao Zhili wrote:
>
> > ---
> > libavutil/error.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/libavutil/error.h b/libavutil/error.h
> > index 71df4da..8a35fef 100644
> > --- a/libavutil/error.h
> > +++ b/libavutil/error.h
> > @@ -116,8 +116,10 @@ static inline char *av_make_error_string(char
> > *errbuf, size_t errbuf_size, int e
> > * Convenience macro, the return value should be used only directly in
> > * function arguments but never stand-alone.
> > */
> > +#ifndef __cplusplus
> > #define av_err2str(errnum) \
> > av_make_error_string((char[AV_ERROR_MAX_STRING_SIZE]){0},
> > AV_ERROR_MAX_STRING_SIZE, errnum)
> > +#endif
> >
> > /**
> > * @}
> > --
> > 2.9.5
> >
> >
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
> NAK
> Chopping off parts of the API because it doesn't play well when included in
> a different language is not a good idea.
> Just do #undef av_make_error_string in your own code.
+1, I don't want a __cplusplus mess in the headers. We can document
them as C99 only, though.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] compound literal in public header file
On Thu, 17 May 2018 00:41:10 +0200
Carl Eugen Hoyos wrote:
> 2018-05-16 11:47 GMT+02:00, Steinar H. Gunderson
> :
> > On Wed, May 16, 2018 at 11:41:23AM +0200, Tobias Rapp wrote:
> >> Yes, I am referring to usage of the libavutil headers in C. If the macro
> >> is
> >> only hidden for C++ and available in C, that would be OK for me. But if
> >> the
> >> static inline function variant would support both C and C++, this would
> >> look
> >> like a solution where C++ users are not forced to implement a more
> >> "integrated" replacement but of course have the option to do so.
> >
> > While we're at it, would it be possible to have extern "C" { for the
> > headers,
> > too? :-) (Wrapped in #ifdef __cplusplus, of course.)
>
> No, because it would give the users of the headers the impression
> that they are tested with c++ which - as you found out - they are not.
That's a pointless argument, because we definitely support C++ API
users, and any breakages are fixed immediately, or are prevented in
patch review in the first place. The presence of C only helper macros
changes nothing about this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hls: tag as AVFMT_TS_DISCONT
On Wed, 16 May 2018 11:45:58 -0700
Aman Gupta wrote:
> On Wed, May 16, 2018 at 11:14 AM, wm4 wrote:
>
> > On Wed, 16 May 2018 10:17:58 -0700
> > Aman Gupta wrote:
> >
> > > From: Aman Gupta
> > >
> > > HLS streams can contain discontinuities. Mark the format as such.
> > >
> > > This triggers various discontinuity fixes in lavf/utils.c and fftools
> > >
> > > Signed-off-by: Aman Gupta
> > > ---
> > > libavformat/hls.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > > index 4ee4be769d..3199b0ac8d 100644
> > > --- a/libavformat/hls.c
> > > +++ b/libavformat/hls.c
> > > @@ -2277,7 +2277,7 @@ AVInputFormat ff_hls_demuxer = {
> > > .long_name = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
> > > .priv_class = &hls_class,
> > > .priv_data_size = sizeof(HLSContext),
> > > -.flags = AVFMT_NOGENSEARCH,
> > > +.flags = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
> > > .read_probe = hls_probe,
> > > .read_header= hls_read_header,
> > > .read_packet= hls_read_packet,
> >
> > I think I'm against this. HLS streams do not typically contain
> > timestamp resets (even if they could). Otherwise you might as well add
> >
>
> mpegts timestamps wrap every 26 hours, so any live continuous HLS broadcast
> is guaranteed to have a timestamp reset.
> Users have reported seeing these resets on various streams (
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-March/226706.html), plus
> the HLS spec itself defines a EXT-X-DISCONTINUITY tag to signal
> discontinuities. So I don't think it's fair to say that discontinuities
> don't typically occur and can be ignored.
Wouldn't it be better to introduce a discontinuity notification then?
Then adding the discont flag would be redundant and could be avoided.
It looks like the HLS tag would map cleanly to this.
>
> > this flag to the Matroska demuxer. Besides, it would break some of my
> > code, which uses this flag as a heuristic to detect mpeg-ts style
> > non-container formats.
> >
>
> I'm working on a patchset with a new -remove_ts_discont option which will
> remove discontinuities and normalize timestamps. This
> option would only kick in for formats marked as AVFMT_TS_DISCONT, so I
> would like to see any formats where timestamp
> discontinuities are possible to be marked as such.
My suggestion would probably not block this.
> Maybe we can add another AVFMT flag that can be used as your heuristic?
Well, my heuristic is probably messed up and nonsense anyway. I'd just
find it nice if this isn't enabled to avoid currently released
versions, but I guess my argument is weak.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hls: tag as AVFMT_TS_DISCONT
On Wed, 16 May 2018 10:17:58 -0700
Aman Gupta wrote:
> From: Aman Gupta
>
> HLS streams can contain discontinuities. Mark the format as such.
>
> This triggers various discontinuity fixes in lavf/utils.c and fftools
>
> Signed-off-by: Aman Gupta
> ---
> libavformat/hls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 4ee4be769d..3199b0ac8d 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2277,7 +2277,7 @@ AVInputFormat ff_hls_demuxer = {
> .long_name = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
> .priv_class = &hls_class,
> .priv_data_size = sizeof(HLSContext),
> -.flags = AVFMT_NOGENSEARCH,
> +.flags = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
> .read_probe = hls_probe,
> .read_header= hls_read_header,
> .read_packet= hls_read_packet,
I think I'm against this. HLS streams do not typically contain
timestamp resets (even if they could). Otherwise you might as well add
this flag to the Matroska demuxer. Besides, it would break some of my
code, which uses this flag as a heuristic to detect mpeg-ts style
non-container formats.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] hevcdec: Miss the location of chroma samples when exporting stream parameters
On Wed, 16 May 2018 15:19:44 +0800
Haihao Xiang wrote:
> Signed-off-by: Haihao Xiang
> ---
> libavcodec/hevcdec.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index c8877626d2..13d868bb4f 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -344,6 +344,11 @@ static void export_stream_params(AVCodecContext *avctx,
> const HEVCParamSets *ps,
> avctx->colorspace = AVCOL_SPC_UNSPECIFIED;
> }
>
> +if (sps->vui.chroma_loc_info_present_flag)
> +avctx->chroma_sample_location =
> sps->vui.chroma_sample_loc_type_top_field + 1;
> +else
> +avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
> +
> if (vps->vps_timing_info_present_flag) {
> num = vps->vps_num_units_in_tick;
> den = vps->vps_time_scale;
Wouldn't this prevent an API user from setting the field to a container
value as a fallback? Although maybe that's not necessary because
there's an "unspecified" special value anyway.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] compound literal in public header file
On Wed, 16 May 2018 11:47:36 +0200
"Steinar H. Gunderson" wrote:
> On Wed, May 16, 2018 at 11:41:23AM +0200, Tobias Rapp wrote:
> > Yes, I am referring to usage of the libavutil headers in C. If the macro is
> > only hidden for C++ and available in C, that would be OK for me. But if the
> > static inline function variant would support both C and C++, this would look
> > like a solution where C++ users are not forced to implement a more
> > "integrated" replacement but of course have the option to do so.
>
> While we're at it, would it be possible to have extern "C" { for the headers,
> too? :-) (Wrapped in #ifdef __cplusplus, of course.)
That would probably not harm anything. It's pretty standard for C
libraries to have this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] compound literal in public header file
On Wed, 16 May 2018 17:58:46 +0800
Zhao Zhili wrote:
> On 2018年05月16日 17:47, Steinar H. Gunderson wrote:
> > On Wed, May 16, 2018 at 11:41:23AM +0200, Tobias Rapp wrote:
> >> Yes, I am referring to usage of the libavutil headers in C. If the macro is
> >> only hidden for C++ and available in C, that would be OK for me. But if the
> >> static inline function variant would support both C and C++, this would
> >> look
> >> like a solution where C++ users are not forced to implement a more
> >> "integrated" replacement but of course have the option to do so.
> > While we're at it, would it be possible to have extern "C" { for the
> > headers,
> > too? :-) (Wrapped in #ifdef __cplusplus, of course.)
> >
> > /* Steinar */
>
> This is what I thought. Although let the C++ programmer use it and fail
> and learn something new is cool, I prefer less surprise.
More people should be educated about which of C and C++ is the inferior
language.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: Fix errors in version script list
On Wed, 16 May 2018 10:13:44 +0200
Hendrik Leppkes wrote:
> On Wed, May 16, 2018 at 2:39 AM, Peter Bennett wrote:
> > From: Peter Bennett
> >
> > libavformat.v has url_open, url_close and url_write. These
> > should be ffurl_ in each case.
> > ---
> > libavformat/libavformat.v | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/libavformat.v b/libavformat/libavformat.v
> > index cf2055f..7df12d3 100644
> > --- a/libavformat/libavformat.v
> > +++ b/libavformat/libavformat.v
> > @@ -6,9 +6,9 @@ LIBAVFORMAT_MAJOR {
> > ffurl_seek;
> > ffurl_size;
> > ffurl_protocol_next;
> > -url_open;
> > -url_close;
> > -url_write;
> > +ffurl_open;
> > +ffurl_close;
> > +ffurl_write;
> > url_*;
> > ff_timefilter_destroy;
> > ff_timefilter_new;
> > --
> > 2.7.4
> >
>
> libavformat.v contains none of these entries anymore.
Also the url API (why does it even exist?) hasn't been public API for
years.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][ALT PATCHES] Code of Conduct Enforcement
On Wed, 16 May 2018 15:44:36 +0100 Derek Buitenhuis wrote: > On Wed, May 16, 2018 at 3:25 PM, James Almer wrote: > > I think the issue is not the lack of clear enforcement rules, but a lack > > of a proactive enforcer, be it a person or a body. The CoC has done > > nothing but give people something to say when they want to be passive > > aggressive in a discussion. > > Changing it to a more strict one like Videolan's will not affect that. > > > > This lack of interest you mentioned is exactly the issue. If it all > > depends on the coordinated action and involvement of a dozen or so > > developers, then nothing will happen. As i said, the one time action was > > called for a developer's behavior, no voter showed up. > > So how does one fix a toxic environment where most of the members > don't care, or don't even aknowledge it's an issue? Let it hemmorage > people, keeping a few here and there who can "take it" (which IMO is > a load of gross gatekeeping BS)? > > (Yes, As far as I'm concerned, staying away / not replying / doing nothing > si just an implicit way of saying "I'm fine with the community as it is". ) > > I actually don't have much of a solution to the "nobody willing to step > up" problem, though, no. The offical documents should at least reflect > that reality then, though. A CoC that is ignored, is not a CoC. True, in that case it's better to remove the CoC because it's completely misleading about our community. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Tue, 15 May 2018 17:15:05 +0100
Rostislav Pehlivanov wrote:
> On 15 May 2018 at 15:55, wm4 wrote:
>
> > On Mon, 14 May 2018 18:26:35 -0400
> > Patrick Keroulas wrote:
> >
> > > Signed-off-by: Patrick Keroulas
> > > ---
> > > doc/APIchanges | 3 +++
> > > libavcodec/avcodec.h | 8
> > > libavcodec/version.h | 4 ++--
> > > 3 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index bbefc83..d06868e 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> > >
> > > API changes, most recent first:
> > >
> > > +2018-05-xx - xx - lavc 58.20.100 - avcodec.h
> > > + Add AV_PKT_FLAG_TOP_FIELD and AV_PKT_FLAG_BOTTOM_FIELD.
> > > +
> > > 2018-05-xx - xx - lavu 56.18.101 - hwcontext_cuda.h
> > >Add AVCUDADeviceContext.stream.
> > >
> > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > index fb0c6fa..14811be 100644
> > > --- a/libavcodec/avcodec.h
> > > +++ b/libavcodec/avcodec.h
> > > @@ -1480,6 +1480,14 @@ typedef struct AVPacket {
> > > */
> > > #define AV_PKT_FLAG_DISPOSABLE 0x0010
> > >
> > > +/**
> > > + * The packet contains a top field.
> > > + */
> > > +#define AV_PKT_FLAG_TOP_FIELD 0x0020
> > > +/**
> > > + * The packet contains a bottom field.
> > > + */
> > > +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040
> > >
> > > enum AVSideDataParamChangeFlags {
> > > AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
> > > diff --git a/libavcodec/version.h b/libavcodec/version.h
> > > index 3fda743..b9752ce 100644
> > > --- a/libavcodec/version.h
> > > +++ b/libavcodec/version.h
> > > @@ -28,8 +28,8 @@
> > > #include "libavutil/version.h"
> > >
> > > #define LIBAVCODEC_VERSION_MAJOR 58
> > > -#define LIBAVCODEC_VERSION_MINOR 19
> > > -#define LIBAVCODEC_VERSION_MICRO 101
> > > +#define LIBAVCODEC_VERSION_MINOR 20
> > > +#define LIBAVCODEC_VERSION_MICRO 100
> > >
> > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR,
> > >
> > \
> > >
> > LIBAVCODEC_VERSION_MINOR, \
> >
> > So far we could avoid codec-specific packet flags, and I think it
> > should stay this way. Maybe make it side data, something with naming
> > specific to the bitpacked codec. Or alternatively, if this codec
> > is 100% RTP specific and there's no such thing as standard bitpacked
> > packets (e.g. muxed in other files etc.), add it to the packet
> > directly. The RTP code "repacks" it already on the libavformat side
> > anyway.
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
> This codec isn't RTP specific, its the same as V210. There are no flags in
> the bitstream, its just a sequence of packed pixels. And just like v210
> there's a standard for what packets need to look like (rfc4175, and
> unfortunately it does specify the fields need to be separate), so packing 2
> fields in the muxer isn't really an option.
Then why are there separate bitpacked and v210 decoders/codec_ids?
> Side data seems a bit of an overkill for a flag. I'd say the field flags
> are not codec specific as some raw codecs and containers can signal fields
> per packet. So I don't really mind them.
You want codec specific flags to accumulate in AVPacket.flags? Can't way
until we change the flags field to int128_t.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field
On Mon, 14 May 2018 18:26:35 -0400
Patrick Keroulas wrote:
> Signed-off-by: Patrick Keroulas
> ---
> doc/APIchanges | 3 +++
> libavcodec/avcodec.h | 8
> libavcodec/version.h | 4 ++--
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index bbefc83..d06868e 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> +2018-05-xx - xx - lavc 58.20.100 - avcodec.h
> + Add AV_PKT_FLAG_TOP_FIELD and AV_PKT_FLAG_BOTTOM_FIELD.
> +
> 2018-05-xx - xx - lavu 56.18.101 - hwcontext_cuda.h
>Add AVCUDADeviceContext.stream.
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index fb0c6fa..14811be 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1480,6 +1480,14 @@ typedef struct AVPacket {
> */
> #define AV_PKT_FLAG_DISPOSABLE 0x0010
>
> +/**
> + * The packet contains a top field.
> + */
> +#define AV_PKT_FLAG_TOP_FIELD 0x0020
> +/**
> + * The packet contains a bottom field.
> + */
> +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040
>
> enum AVSideDataParamChangeFlags {
> AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 3fda743..b9752ce 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,8 +28,8 @@
> #include "libavutil/version.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 58
> -#define LIBAVCODEC_VERSION_MINOR 19
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MINOR 20
> +#define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
So far we could avoid codec-specific packet flags, and I think it
should stay this way. Maybe make it side data, something with naming
specific to the bitpacked codec. Or alternatively, if this codec
is 100% RTP specific and there's no such thing as standard bitpacked
packets (e.g. muxed in other files etc.), add it to the packet
directly. The RTP code "repacks" it already on the libavformat side
anyway.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][ALT PATCHES] Code of Conduct Enforcement
On Mon, 14 May 2018 17:50:25 +0100 Derek Buitenhuis wrote: > Hello all. > > This is a little rambling / stream of thought, but take it as you will, > and perhaps some discussion or change comes of it. Or, more likely, personal > attacks, flames, and no change. Or 1 few will reply and then the thread will > die and people will go on like it never happened. Sorry to be pessimistic, > but history speaks for itself. > > Currently this list, and IRC, is a terrible place for developers and users. > Harassment is tolerated, and the CoC has never been enforced. There are > absolutely no repercussions as it currently stands. When develpers see others > being abused, do nothing and then continue to act and interact with the > abuses as if nothing happened, in the future. > > This current set up of everybody flaming each other endlessly, and endless > harassment merely lets abuse proliferate, because nobody sticks up for each > other and no abusers have ever been removed, ever, in the history of FFmpeg. > Not one thing has changed, ever, since the introduction of the CoC. It is > an absolute failure by all metrics. > > I know I am not alone having been driven away by such behavior. Even when > I started to contribute (a little, not much) again, upon joining IRC, for > example, I was immediately attacked. This is not just happening to one > person though. > > To be honest, I am not really sure what can be done. Large portions of > the list simply do not support anti-abuse measures at all. Even the > concept of them. How does one manage to implement them without support > of even 50% of a community? VideoLAN managed to do this but it took a lot > of formal stuff on their board and people quitting to get it done. j-b > is some sort of wizard. > > I know my opinion is not worth much, since I am now more or less an outsider > since 2015, but maybe someone cares about this stuff, too. > > So, I present to you two possible options: > > 1. Implement a formal CoC enforcement system. This has been mostly > copypasted >from VideoLAN's, and is meant as more of a blueprint. This will no > doubt >be controversial. > 2. Remove the CoC. If you're not going to enforce it, ever, there is > literally >no point in having one. I know some members of this community object > to the >very notion of a CoC, so this should please them, I am sure. > > I'm sure this will be a civil discussion. 1, please. I don't want PC reeducation camps, but not having a kindergarten would be nice. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] qsv: Use the installed mfx include headers if possibile
On Fri, 11 May 2018 11:38:52 +0100
Mark Thompson wrote:
> On 11/05/18 08:52, Haihao Xiang wrote:
> > Currently an extra copy of mfx include headers from
> > {MSDK_INSTALL_PREFIX}/include
> > to {MSDK_INSTALL_PREFIX}/include/mfx is required when using pkg-config for
> > libmfx detection. This fix checks the installed mfx include headers first,
> > and falls back to the legacy way if that fails
> >
> > Signed-off-by: Haihao Xiang
> > ---
> > configure| 3 ++-
> > fftools/ffmpeg_qsv.c | 5 +
> > libavcodec/qsv.c | 10 ++
> > libavcodec/qsv.h | 4
> > libavcodec/qsv_internal.h| 4
> > libavcodec/qsvdec.c | 4
> > libavcodec/qsvdec.h | 4
> > libavcodec/qsvdec_h2645.c| 4
> > libavcodec/qsvdec_other.c| 4
> > libavcodec/qsvenc.c | 5 +
> > libavcodec/qsvenc.h | 4
> > libavcodec/qsvenc_h264.c | 4
> > libavcodec/qsvenc_hevc.c | 4
> > libavcodec/qsvenc_jpeg.c | 4
> > libavcodec/qsvenc_mpeg2.c| 4
> > libavfilter/qsvvpp.h | 4
> > libavfilter/vf_deinterlace_qsv.c | 4
> > libavfilter/vf_scale_qsv.c | 4
> > libavutil/hwcontext_opencl.c | 4
> > libavutil/hwcontext_qsv.c| 4
> > libavutil/hwcontext_qsv.h| 4
> > 21 files changed, 90 insertions(+), 1 deletion(-)
>
> I don't think it's a good idea to put an #ifdef like this in every file.
+1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default
On Fri, 11 May 2018 01:49:58 +0200 James Darnley wrote: > ... Security in ffmpeg sure is weird. On one hand we get all kinds of crazy stuff that's supposed to mitigate... something (like whitelists), on the other hand we get this. > I haven't tried to stand in the way of other bad changes to ffmpeg (like > the fact that the flac muxer will now mux video streams) and I won't try > to stand in the way of this one. That sounds insane, but it's probably about cover art. Only ffmpeg is insane enough to treat cover art as single-frame video stream. (And as an API user I've really tried hard to make this work, but it remains a painful special case.) So, not sure why you claim it's a "bad change" to add support for that in the flac muxer. It merely follows the ffmpeg API. Anyway, I guess D. B. is right and this thread is a fucking waste of time, so goodbye. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default
On Fri, 11 May 2018 00:53:16 +0100 Rostislav Pehlivanov wrote: > On 11 May 2018 at 00:44, wm4 wrote: > > > On Fri, 11 May 2018 00:41:20 +0100 > > Derek Buitenhuis wrote: > > > > > On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol wrote: > > > > I do not have time to explain security basics. > > > > Get a decent book and read it from a start to an end. > > > > > > Speaking frankly for a second: Why do people put up with this sort of > > > crud on this > > > mailing list? > > > > > > Insult-laden fact-less response. It's insane. It's like I'm flashing > > > back to ffmpeg-devel > > > in 2006. > > > > Because there are no consequences > > > There are consequeces, the consequences being broken design and/or features > > > no leadership > > > Just because there are many people determining what happens doesn't mean no > one is leading. > > > no conflict arbitration. > > > > There is conflict arbitration, we're doing it right now in fact. Yeah, we can just flame each other as hard as we can every time, until someone gives up out of boredom or disgust. This is why ffmpeg is one of the worst open source projects around. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default
On Fri, 11 May 2018 00:41:20 +0100 Derek Buitenhuis wrote: > On Fri, May 11, 2018 at 12:35 AM, Paul B Mahol wrote: > > I do not have time to explain security basics. > > Get a decent book and read it from a start to an end. > > Speaking frankly for a second: Why do people put up with this sort of > crud on this > mailing list? > > Insult-laden fact-less response. It's insane. It's like I'm flashing > back to ffmpeg-devel > in 2006. Because there are no consequences, no leadership, no conflict arbitration. It's a joke of a project. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default
On Fri, 11 May 2018 00:21:37 +0100 Rostislav Pehlivanov wrote: > On 10 May 2018 at 23:27, Paul B Mahol wrote: > > > On 5/11/18, wm4 wrote: > > > On Thu, 10 May 2018 16:44:59 +0100 > > > Derek Buitenhuis wrote: > > > > > >> These demuxers have probes that mainly probe based on file extension, > > >> and map to codec IDs that render text as video. The result is that > > >> ffmpeg will, by default, happily render, for example, .txt files > > >> as images. This is not exactly a good security practice, an only > > >> makes it easier for potential attackers to gain the contents of > > >> system files. > > >> > > >> Disable building these by default. > > >> > > >> Signed-off-by: Derek Buitenhuis > > >> --- > > > > > > +1 > > > > > > You should send a patch that disables all those useless game demuxers > > > too. They only cause security issues and bloated library sizes. > > > > Against. > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > I agree with Paul, game demuxers are useful, don't bloat much and can be > fixed. Experience shows that it's always the obscure features which cause security issues. Regarding the bloat: these small things add up a lot, and suddenly you have hundreds of demuxers. It's hard to filter them out manually, and why make each user do that? Many of these game formats in particular probably have something like under a dozen files in the universe that exist at all (such as the files included in a particular game release). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [RFC][PATCH] configure: Disable unsafe demuxers by default
On Thu, 10 May 2018 16:44:59 +0100 Derek Buitenhuis wrote: > These demuxers have probes that mainly probe based on file extension, > and map to codec IDs that render text as video. The result is that > ffmpeg will, by default, happily render, for example, .txt files > as images. This is not exactly a good security practice, an only > makes it easier for potential attackers to gain the contents of > system files. > > Disable building these by default. > > Signed-off-by: Derek Buitenhuis > --- +1 You should send a patch that disables all those useless game demuxers too. They only cause security issues and bloated library sizes. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] libavcodec: v4l2m2m: output AVDRMFrameDescriptor
On Thu, 10 May 2018 01:24:42 +0200
Jorge Ramirez-Ortiz wrote:
> On 05/09/2018 04:11 PM, Mark Thompson wrote:
> > On 09/05/18 08:57, Jorge Ramirez-Ortiz wrote:
> >> On 05/09/2018 01:33 AM, Mark Thompson wrote:
> >>> diff --git a/libavcodec/v4l2_m2m_dec.c b/libavcodec/v4l2_m2m_dec.c
> >>> index ed5193ecc1..2b33badb08 100644
> >>> --- a/libavcodec/v4l2_m2m_dec.c
> >>> +++ b/libavcodec/v4l2_m2m_dec.c
> >>> @@ -23,12 +23,18 @@
> >>> #include
> >>> #include
> >>> +
> >>> +#include "libavutil/hwcontext.h"
> >>> +#include "libavutil/hwcontext_drm.h"
> >>> #include "libavutil/pixfmt.h"
> >>> #include "libavutil/pixdesc.h"
> >>> #include "libavutil/opt.h"
> >>> #include "libavcodec/avcodec.h"
> >>> #include "libavcodec/decode.h"
> >>> +#include "libavcodec/hwaccel.h"
> >>> +#include "libavcodec/internal.h"
> >>> +
> >>> #include "v4l2_context.h"
> >>> #include "v4l2_m2m.h"
> >>> #include "v4l2_fmt.h"
> >>> @@ -183,6 +189,15 @@ static av_cold int v4l2_decode_init(AVCodecContext
> >>> *avctx)
> >>> capture->av_codec_id = AV_CODEC_ID_RAWVIDEO;
> >>> capture->av_pix_fmt = avctx->pix_fmt;
> >>> + /* the client requests the codec to generate DRM frames:
> >>> + * - data[0] will therefore point to the returned
> >>> AVDRMFrameDescriptor
> >>> + * check the ff_v4l2_buffer_to_avframe conversion function.
> >>> + * - the DRM frame format is passed in the DRM frame descriptor
> >>> layer.
> >>> + * check the v4l2_get_drm_frame function.
> >>> + */
> >>> + if (avctx->pix_fmt == AV_PIX_FMT_DRM_PRIME)
> >>> + s->output_drm = 1;
> >>> +
> >>> ret = ff_v4l2_m2m_codec_init(avctx);
> >>> if (ret) {
> >>> av_log(avctx, AV_LOG_ERROR, "can't configure decoder\n");
> >>> @@ -202,6 +217,11 @@ static const AVOption options[] = {
> >>> { NULL},
> >>> };
> >> As a follow up to your comment on pixel format negotiation
> >> (AvCodecContext.getformat), notice that this is a tentative request from
> >> the user to select a pixel format.
> >> The actual pixel format negotiation - where the decoder will select the
> >> pixel format- will happen later during v4l2_try_start.
> > Indeed. get_format() will have to be called during the pixel format
> > negotiation so that the user can pick between whatever the supported
> > software format is (NV12, NV21, YUV420P P010, whatever) or the DRM-PRIME
> > object hardware format (if supported).
>
> but in the use case that we are trying to implement, it is the user -
> not the codec - the one that has to specifically request DRM support to
> libavcodec since the v4l2m2m decoder can provide either of them
> indistinctly.
>
> it is just not clear to me how the libavcodec client can request the DRM
> pix_fmt if not tentatively setting the pix_fmt during v4l2_decode_init.
>
>
> >
> > AVCodecContext.pix_fmt is intended to be set by the decoder to say what
> > pix_fmt it intends to produce (though even in that role it's highly dubious
> > given threaded decoders and stream changes). For historical reasons it is
> > also allowed to be set externally (because of libavformat interactions),
> > but this shouldn't be used for configuration.
> >
> >> This change enables the v4l2m2m decoder to output either dmabuf
> >> descriptors to be consumed by a DRM or video frame formats to be consumed
> >> by SDL (for instance).
> >> As an example, these changes have been tested with ffplay (SDL based
> >> display) and a simple DRM application [1]
> >>
> >> Lukas tested with other tools.
> >>
> >> [1]https://github.com/baylibre/ffmpeg-drm
> > We should make this usable in the ffmpeg application too. The DRM object
> > format is works fine in ffmpeg already with Rockchip decoder (consumed by
> > the hwmap/hwdownload filters, or by mapping to OpenCL), but that doesn't
> > need the format selection part. (There are also kmsgrab and VAAPI, but
> > those aren't making DRM PRIME frames directly at a decoder.) I believe
> > this should be straightforward with a small modification to get_format() in
> > ffmpeg.c to accept AV_CODEC_HW_CONFIG_METHOD_INTERNAL, I can look at this
> > once we have a codec which will need it.
>
> so what would be the sequence of calls for an libavcodec client to
> request the DRM format?
> https://github.com/BayLibre/ffmpeg-drm/blob/master/main.c#L598
Like in 100% of all existing cases in ffmpeg: set a get_format
callback, and return the format from it.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/6] avutil/hwcontext_cuda: add CUstream in cuda hwctx
On Tue, 8 May 2018 17:48:21 +0200
Timo Rothenpieler wrote:
> Am 08.05.2018 um 17:26 schrieb wm4:
> > On Tue, 8 May 2018 15:31:29 +0200
> > Timo Rothenpieler wrote:
> >
> >> ---
> >> configure | 6 --
> >> doc/APIchanges | 3 ++-
> >> libavutil/hwcontext_cuda.c | 3 +++
> >> libavutil/hwcontext_cuda.h | 1 +
> >> libavutil/version.h| 2 +-
> >> 5 files changed, 11 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index 7c143238a8..cae8a235a4 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -5887,8 +5887,10 @@ check_type "va/va.h va/va_enc_vp9.h"
> >> "VAEncPictureParameterBufferVP9"
> >> check_type "vdpau/vdpau.h" "VdpPictureInfoHEVC"
> >>
> >> if ! disabled ffnvcodec; then
> >> -check_pkg_config ffnvcodec "ffnvcodec >= 8.0.14.1" \
> >> -"ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> >> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" ""
> >> +check_pkg_config ffnvcodec "ffnvcodec >= 8.1.24.2" \
> >> + "ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> >> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" "" || \
> >> +{ test_pkg_config ffnvcodec_tmp "ffnvcodec < 8.1" "" "" &&
> >> check_pkg_config ffnvcodec "ffnvcodec >= 8.0.14.2" \
> >> + "ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> >> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" ""; }
> >> fi
> >>
> >> check_cpp_condition winrt windows.h
> >> "!WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)"
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index f8ae6b0433..7a0a8522f9 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -15,8 +15,9 @@ libavutil: 2017-10-21
> >>
> >> API changes, most recent first:
> >>
> >> -2018-05-xx - xx - lavu 56.19.100 - hwcontext_cuda.h
> >> +2018-05-xx - xx, xx - lavu 56.19.100/101 -
> >> hwcontext_cuda.h
> >> Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> >> + Add AVCUDADeviceContext.stream.
> >>
> >> 2018-04-xx - xx - lavu 56.18.100 - pixdesc.h
> >> Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
> >> diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
> >> index b0b4bf24ae..8024eec79d 100644
> >> --- a/libavutil/hwcontext_cuda.c
> >> +++ b/libavutil/hwcontext_cuda.c
> >> @@ -395,6 +395,9 @@ static int cuda_device_create(AVHWDeviceContext *ctx,
> >> const char *device,
> >> goto error;
> >> }
> >>
> >> +// Setting stream to NULL will make functions automatically use the
> >> default CUstream
> >> +hwctx->stream = NULL;
> >> +
> >> cu->cuCtxPopCurrent(&dummy);
> >>
> >> hwctx->internal->is_allocated = 1;
> >> diff --git a/libavutil/hwcontext_cuda.h b/libavutil/hwcontext_cuda.h
> >> index 19accbd9be..cd797ae920 100644
> >> --- a/libavutil/hwcontext_cuda.h
> >> +++ b/libavutil/hwcontext_cuda.h
> >> @@ -41,6 +41,7 @@ typedef struct AVCUDADeviceContextInternal
> >> AVCUDADeviceContextInternal;
> >>*/
> >> typedef struct AVCUDADeviceContext {
> >> CUcontext cuda_ctx;
> >> +CUstream stream;
> >> AVCUDADeviceContextInternal *internal;
> >> } AVCUDADeviceContext;
> >>
> >> diff --git a/libavutil/version.h b/libavutil/version.h
> >> index 84409b1d69..f84ec89154 100644
> >> --- a/libavutil/version.h
> >> +++ b/libavutil/version.h
> >> @@ -80,7 +80,7 @@
> >>
> >> #define LIBAVUTIL_VERSION_MAJOR 56
> >> #define LIBAVUTIL_VERSION_MINOR 19
> >> -#define LIBAVUTIL_VERSION_MICRO 100
> >> +#define LIBAVUTIL_VERSION_MICRO 101
> >>
> >> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >> LIBAVUTIL_VERSION_MINOR,
> >> \
> >
> > What is this?
>
> https://docs.nvidia.com/cuda/cuda-driver-api/stream-sync-behavior.html
>
> It allows asynchronous processing of CUDA workloads. The next couple
> patches make use of it.
> There's no change in behaviour if it remains unset/NULL, but if you set
> one, the workload won't block the main CUDA stream so you can do
> multiple transcode sessions in the same application without blocking one
> another.
>
Could probably be documented.
It seems a bit strange that this is per device. Wouldn't it be per
operation?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_cuda: add AVCUDAFramesContext and AVCUDAFramesContext.flags
On Tue, 8 May 2018 17:43:49 +0200 Timo Rothenpieler wrote: > >> -frame->buf[0] = av_buffer_pool_get(ctx->pool); > >> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE) > >> +frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0); > >> +else > >> +frame->buf[0] = av_buffer_pool_get(ctx->pool); > >> + > > > > Is this really needed? Because at least videotoolbox also lets the > > decoder allocate frames, and allocates the "dummy" buffers outside of > > the hwcontext. (I don't quite remember how it works.) > > You mean compared to just leaving buf[0] empty? > No, compared to how the videotoolbox code does things. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/6] avutil/hwcontext_cuda: add CUstream in cuda hwctx
On Tue, 8 May 2018 15:31:29 +0200
Timo Rothenpieler wrote:
> ---
> configure | 6 --
> doc/APIchanges | 3 ++-
> libavutil/hwcontext_cuda.c | 3 +++
> libavutil/hwcontext_cuda.h | 1 +
> libavutil/version.h| 2 +-
> 5 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 7c143238a8..cae8a235a4 100755
> --- a/configure
> +++ b/configure
> @@ -5887,8 +5887,10 @@ check_type "va/va.h va/va_enc_vp9.h"
> "VAEncPictureParameterBufferVP9"
> check_type "vdpau/vdpau.h" "VdpPictureInfoHEVC"
>
> if ! disabled ffnvcodec; then
> -check_pkg_config ffnvcodec "ffnvcodec >= 8.0.14.1" \
> -"ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" ""
> +check_pkg_config ffnvcodec "ffnvcodec >= 8.1.24.2" \
> + "ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" "" || \
> +{ test_pkg_config ffnvcodec_tmp "ffnvcodec < 8.1" "" "" &&
> check_pkg_config ffnvcodec "ffnvcodec >= 8.0.14.2" \
> + "ffnvcodec/nvEncodeAPI.h ffnvcodec/dynlink_cuda.h
> ffnvcodec/dynlink_cuviddec.h ffnvcodec/dynlink_nvcuvid.h" ""; }
> fi
>
> check_cpp_condition winrt windows.h
> "!WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)"
> diff --git a/doc/APIchanges b/doc/APIchanges
> index f8ae6b0433..7a0a8522f9 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,8 +15,9 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> -2018-05-xx - xx - lavu 56.19.100 - hwcontext_cuda.h
> +2018-05-xx - xx, xx - lavu 56.19.100/101 - hwcontext_cuda.h
>Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> + Add AVCUDADeviceContext.stream.
>
> 2018-04-xx - xx - lavu 56.18.100 - pixdesc.h
>Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
> diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
> index b0b4bf24ae..8024eec79d 100644
> --- a/libavutil/hwcontext_cuda.c
> +++ b/libavutil/hwcontext_cuda.c
> @@ -395,6 +395,9 @@ static int cuda_device_create(AVHWDeviceContext *ctx,
> const char *device,
> goto error;
> }
>
> +// Setting stream to NULL will make functions automatically use the
> default CUstream
> +hwctx->stream = NULL;
> +
> cu->cuCtxPopCurrent(&dummy);
>
> hwctx->internal->is_allocated = 1;
> diff --git a/libavutil/hwcontext_cuda.h b/libavutil/hwcontext_cuda.h
> index 19accbd9be..cd797ae920 100644
> --- a/libavutil/hwcontext_cuda.h
> +++ b/libavutil/hwcontext_cuda.h
> @@ -41,6 +41,7 @@ typedef struct AVCUDADeviceContextInternal
> AVCUDADeviceContextInternal;
> */
> typedef struct AVCUDADeviceContext {
> CUcontext cuda_ctx;
> +CUstream stream;
> AVCUDADeviceContextInternal *internal;
> } AVCUDADeviceContext;
>
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 84409b1d69..f84ec89154 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -80,7 +80,7 @@
>
> #define LIBAVUTIL_VERSION_MAJOR 56
> #define LIBAVUTIL_VERSION_MINOR 19
> -#define LIBAVUTIL_VERSION_MICRO 100
> +#define LIBAVUTIL_VERSION_MICRO 101
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> LIBAVUTIL_VERSION_MINOR, \
What is this?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avutil/hwcontext_cuda: add AVCUDAFramesContext and AVCUDAFramesContext.flags
On Mon, 7 May 2018 23:46:48 +0200
Timo Rothenpieler wrote:
> Frames can be mapped from nvdec/cuvid, not needing any actual memory
> allocation, but all other features of the hw_frames_ctx.
> Hence the dummy-mode, which does not allocate any (notable amounts of)
> memory but otherwise behaves the exact same.
> ---
> doc/APIchanges | 3 +++
> libavutil/hwcontext_cuda.c | 12 +++-
> libavutil/hwcontext_cuda.h | 22 +-
> libavutil/version.h| 2 +-
> 4 files changed, 36 insertions(+), 3 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index ede5b186ae..82ec888fd8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> +2018-05-xx - xx - lavu 56.19.100 - hwcontext.h
> + Add AVCUDAFramesContext and AVCUDAFramesContext.flags.
> +
> 2018-04-xx - xx - lavu 56.18.100 - pixdesc.h
>Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
>
> diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
> index 37827a770c..b0b4bf24ae 100644
> --- a/libavutil/hwcontext_cuda.c
> +++ b/libavutil/hwcontext_cuda.c
> @@ -161,6 +161,7 @@ static int cuda_frames_init(AVHWFramesContext *ctx)
>
> static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
> {
> +AVCUDAFramesContext *frctx = ctx->hwctx;
> int aligned_width;
> int width_in_bytes = ctx->width;
>
> @@ -171,7 +172,11 @@ static int cuda_get_buffer(AVHWFramesContext *ctx,
> AVFrame *frame)
> }
> aligned_width = FFALIGN(width_in_bytes, CUDA_FRAME_ALIGNMENT);
>
> -frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
> +frame->buf[0] = av_buffer_create(NULL, 0, NULL, NULL, 0);
> +else
> +frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +
Is this really needed? Because at least videotoolbox also lets the
decoder allocate frames, and allocates the "dummy" buffers outside of
the hwcontext. (I don't quite remember how it works.)
> if (!frame->buf[0])
> return AVERROR(ENOMEM);
>
> @@ -210,6 +215,10 @@ static int cuda_get_buffer(AVHWFramesContext *ctx,
> AVFrame *frame)
> frame->width = ctx->width;
> frame->height = ctx->height;
>
> +// they're pointing to invalid memory, dangerous to leave set
> +if (frctx->flags & AV_CUDA_HWFRAMES_DUMMY_MODE)
> +frame->data[0] = frame->data[1] = frame->data[2] = NULL;
> +
> return 0;
> }
>
> @@ -402,6 +411,7 @@ const HWContextType ff_hwcontext_type_cuda = {
> .name = "CUDA",
>
> .device_hwctx_size= sizeof(AVCUDADeviceContext),
> +.frames_hwctx_size= sizeof(AVCUDAFramesContext),
> .frames_priv_size = sizeof(CUDAFramesContext),
>
> .device_create= cuda_device_create,
> diff --git a/libavutil/hwcontext_cuda.h b/libavutil/hwcontext_cuda.h
> index 12dae8449e..19accbd9be 100644
> --- a/libavutil/hwcontext_cuda.h
> +++ b/libavutil/hwcontext_cuda.h
> @@ -45,7 +45,27 @@ typedef struct AVCUDADeviceContext {
> } AVCUDADeviceContext;
>
> /**
> - * AVHWFramesContext.hwctx is currently not used
> + * This struct is allocated as AVHWFramesContext.hwctx
> */
> +typedef struct AVCUDAFramesContext {
> +/**
> + * Special implementation-specific flags.
> + *
> + * May be set by the user before calling av_hwframe_ctx_init().
> + */
> +int flags;
> +} AVCUDAFramesContext;
> +
> +/**
> + * No actual allocation will happen, but otherwise behaves like normal.
> + *
> + * This is to be used if a AVHWFramesContext is required, but the actual
> + * allocation is happening outside of it.
> + *
> + * The resulting AVFrames will be identical to normal frames, except for
> + * their data[] pointers being NULL and the AVBufferRef in buf[0] being
> + * set but containing no notable allocation of memory.
> + */
> +#define AV_CUDA_HWFRAMES_DUMMY_MODE (1 << 0)
>
> #endif /* AVUTIL_HWCONTEXT_CUDA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 5185454d9b..84409b1d69 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 56
> -#define LIBAVUTIL_VERSION_MINOR 18
> +#define LIBAVUTIL_VERSION_MINOR 19
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/6] avcodec/nvdec: avoid needless copy of output frame
On Tue, 8 May 2018 15:31:28 +0200
Timo Rothenpieler wrote:
> Replaces the data pointers with the mapped cuvid ones.
> Adds buffer_refs to the frame to ensure the needed contexts stay alive
> and the cuvid idx stays allocated.
> Adds another buffer_ref to unmap the frame when it's unreferenced itself.
> ---
> libavcodec/nvdec.c | 83 +-
> 1 file changed, 60 insertions(+), 23 deletions(-)
>
> diff --git a/libavcodec/nvdec.c b/libavcodec/nvdec.c
> index ab3cb88b27..d98f9dd95e 100644
> --- a/libavcodec/nvdec.c
> +++ b/libavcodec/nvdec.c
> @@ -308,7 +308,7 @@ int ff_nvdec_decode_init(AVCodecContext *avctx)
> params.CodecType = cuvid_codec_type;
> params.ChromaFormat= cuvid_chroma_format;
> params.ulNumDecodeSurfaces = frames_ctx->initial_pool_size;
> -params.ulNumOutputSurfaces = 1;
> +params.ulNumOutputSurfaces = frames_ctx->initial_pool_size;
>
> ret = nvdec_decoder_create(&ctx->decoder_ref, frames_ctx->device_ref,
> ¶ms, avctx);
> if (ret < 0) {
> @@ -354,6 +354,32 @@ static void nvdec_fdd_priv_free(void *priv)
> av_freep(&priv);
> }
>
> +static void nvdec_unmap_mapped_frame(void *opaque, uint8_t *data)
> +{
> +NVDECFrame *unmap_data = (NVDECFrame*)data;
> +NVDECDecoder *decoder = (NVDECDecoder*)unmap_data->decoder_ref->data;
> +CUdeviceptr devptr = (CUdeviceptr)opaque;
> +CUresult err;
> +CUcontext dummy;
> +
> +err = decoder->cudl->cuCtxPushCurrent(decoder->cuda_ctx);
> +if (err != CUDA_SUCCESS) {
> +av_log(NULL, AV_LOG_ERROR, "cuCtxPushCurrent failed\n");
> +goto finish;
> +}
> +
> +err = decoder->cvdl->cuvidUnmapVideoFrame(decoder->decoder, devptr);
> +if (err != CUDA_SUCCESS)
> +av_log(NULL, AV_LOG_ERROR, "cuvidUnmapVideoFrame failed\n");
> +
> +decoder->cudl->cuCtxPopCurrent(&dummy);
> +
> +finish:
> +av_buffer_unref(&unmap_data->idx_ref);
> +av_buffer_unref(&unmap_data->decoder_ref);
> +av_free(unmap_data);
> +}
> +
> static int nvdec_retrieve_data(void *logctx, AVFrame *frame)
> {
> FrameDecodeData *fdd = (FrameDecodeData*)frame->private_ref->data;
> @@ -361,6 +387,7 @@ static int nvdec_retrieve_data(void *logctx, AVFrame
> *frame)
> NVDECDecoder *decoder = (NVDECDecoder*)cf->decoder_ref->data;
>
> CUVIDPROCPARAMS vpp = { .progressive_frame = 1 };
> +NVDECFrame *unmap_data = NULL;
>
> CUresult err;
> CUcontext dummy;
> @@ -383,32 +410,39 @@ static int nvdec_retrieve_data(void *logctx, AVFrame
> *frame)
> goto finish;
> }
>
> -for (i = 0; frame->data[i]; i++) {
> -CUDA_MEMCPY2D cpy = {
> -.srcMemoryType = CU_MEMORYTYPE_DEVICE,
> -.dstMemoryType = CU_MEMORYTYPE_DEVICE,
> -.srcDevice = devptr,
> -.dstDevice = (CUdeviceptr)frame->data[i],
> -.srcPitch = pitch,
> -.dstPitch = frame->linesize[i],
> -.srcY = offset,
> -.WidthInBytes = FFMIN(pitch, frame->linesize[i]),
> -.Height= frame->height >> (i ? 1 : 0),
> -};
> -
> -err = decoder->cudl->cuMemcpy2D(&cpy);
> -if (err != CUDA_SUCCESS) {
> -av_log(logctx, AV_LOG_ERROR, "Error copying decoded frame: %d\n",
> - err);
> -ret = AVERROR_UNKNOWN;
> -goto copy_fail;
> -}
> +unmap_data = av_mallocz(sizeof(*unmap_data));
> +if (!unmap_data) {
> +ret = AVERROR(ENOMEM);
> +goto copy_fail;
> +}
>
> -offset += cpy.Height;
> +frame->buf[1] = av_buffer_create((uint8_t *)unmap_data,
> sizeof(*unmap_data),
> + nvdec_unmap_mapped_frame, (void*)devptr,
> + AV_BUFFER_FLAG_READONLY);
> +if (!frame->buf[1]) {
> +ret = AVERROR(ENOMEM);
> +goto copy_fail;
> }
>
> +unmap_data->idx = cf->idx;
> +unmap_data->idx_ref = av_buffer_ref(cf->idx_ref);
> +unmap_data->decoder_ref = av_buffer_ref(cf->decoder_ref);
> +
> +for (i = 0; frame->linesize[i]; i++) {
> +frame->data[i] = (uint8_t*)(devptr + offset);
> +frame->linesize[i] = pitch;
> +offset += pitch * (frame->height >> (i ? 1 : 0));
> +}
> +
> +goto finish;
> +
> copy_fail:
> -decoder->cvdl->cuvidUnmapVideoFrame(decoder->decoder, devptr);
> +if (!frame->buf[1]) {
> +decoder->cvdl->cuvidUnmapVideoFrame(decoder->decoder, devptr);
> +av_freep(&unmap_data);
> +} else {
> +av_buffer_unref(&frame->buf[1]);
> +}
>
> finish:
> decoder->cudl->cuCtxPopCurrent(&dummy);
> @@ -526,6 +560,7 @@ int ff_nvdec_frame_params(AVCodecContext *avctx,
>int dpb_size)
> {
> AVHWFramesContext *frames_ctx = (AVHWFramesContext*)hw_frames_ctx->data;
> +AVCUDAFramesContext *hwctx = (AVCUDAFramesContext*)frames_ctx-
Re: [FFmpeg-devel] [PATCH] lavf/bluray: translate a read of 0 to EOF
On Sun, 6 May 2018 17:19:44 +0300 Jan Ekström wrote: > Yet another case of forgotten 0 =! EOF translation. The libbluray > documentation specifically mentions that a read of 0 is EOF. > > Reported by Fyr on IRC. > --- > libavformat/bluray.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/bluray.c b/libavformat/bluray.c > index 9282bf9956..635c4f1b87 100644 > --- a/libavformat/bluray.c > +++ b/libavformat/bluray.c > @@ -198,7 +198,7 @@ static int bluray_read(URLContext *h, unsigned char *buf, > int size) > > len = bd_read(bd->bd, buf, size); > > -return len; > +return len == 0 ? AVERROR_EOF : len; > } > > static int64_t bluray_seek(URLContext *h, int64_t pos, int whence) Hilarious, another of those EOf issues. Too bad we didn't just revert that crap. I wonder how many years it'll take until we got all of them. LGTM. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/fic: Check available input space for cursor
On Sat, 5 May 2018 22:47:37 +0200
Michael Niedermayer wrote:
> Fixes: out of array read
> Fixes:
> 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/fic.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/libavcodec/fic.c b/libavcodec/fic.c
> index d7ee370423..6824a5683c 100644
> --- a/libavcodec/fic.c
> +++ b/libavcodec/fic.c
> @@ -337,6 +337,11 @@ static int fic_decode_frame(AVCodecContext *avctx, void
> *data,
> skip_cursor = 1;
> }
>
> +if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
> +av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
> +skip_cursor = 1;
> +}
> +
> /* Slice height for all but the last slice. */
> ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
> if (ctx->slice_h % 16)
No warning needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/vp3: Check that there will be sufficient input for the coded fragments in unpack_superblocks()
On Sat, 5 May 2018 21:10:06 +0200
Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes:
> 6292/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP3_fuzzer-4871218218926080
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer
> ---
> libavcodec/vp3.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> index 1d83753314..fcc19a627b 100644
> --- a/libavcodec/vp3.c
> +++ b/libavcodec/vp3.c
> @@ -543,7 +543,11 @@ static int unpack_superblocks(Vp3DecodeContext *s,
> GetBitContext *gb)
> : s->y_superblock_count);
> int num_coded_frags = 0;
>
> -for (i = sb_start; i < sb_end && get_bits_left(gb) > 0; i++) {
> +for (i = sb_start; i < sb_end; i++) {
> +if (get_bits_left(gb) < ((s->total_num_coded_frags +
> num_coded_frags) >> 2)) {
> +av_log(s->avctx, AV_LOG_ERROR, "Insufficient input data for
> coded fragments\n");
> +return AVERROR_INVALIDDATA;
> +}
> /* iterate through all 16 fragments in a superblock */
> for (j = 0; j < 16; j++) {
> /* if the fragment is in bounds, check its coding status */
The log message is pointless bloat.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: do not write timebase as framerate
On Sat, 5 May 2018 02:35:52 +0200 Carl Eugen Hoyos wrote: > 2018-05-04 15:00 GMT+02:00, wm4 : > > On Sat, 28 Apr 2018 19:24:21 +0200 > > wm4 wrote: > > > >> If the API user doesn't set avg_frame_rate, matroskaenc will write the > >> current timebase as "default duration" for the video track. This makes > >> no sense, because the "default duration" implies the framerate of the > >> video. Since the timebase is forced to 1/1000, this will make the > >> resulting file claim 1000fps. > >> > >> Drop it and don't write the element. It's optional, so it's better not > >> to write it if the framerate is unknown. > >> > >> Strangely does not require FATE changes. > >> --- > >> libavformat/matroskaenc.c | 2 -- > >> 1 file changed, 2 deletions(-) > >> > >> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > >> index 5950b4de44..b7ff1950d3 100644 > >> --- a/libavformat/matroskaenc.c > >> +++ b/libavformat/matroskaenc.c > >> @@ -1304,8 +1304,6 @@ static int mkv_write_track(AVFormatContext *s, > >> MatroskaMuxContext *mkv, > >> if( st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0 > >> && av_cmp_q(av_inv_q(st->avg_frame_rate), st->time_base) > 0) > >> put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, > >> 10LL * st->avg_frame_rate.den / st->avg_frame_rate.num); > >> -else > >> -put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, > >> 10LL * st->time_base.num / st->time_base.den); > >> > >> if (!native_id && > >> ff_codec_get_tag(ff_codec_movvideo_tags, par->codec_id) && > > > > Pushed. > > Good to know that your behaviour hasn't improved. Wow yet another passive aggressive attack against for no reason. I think I'll just block all your mails so I have peace. Just because I participate in this project it doesn't mean I have to tolerate your obnoxious rudeness. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Fri, 4 May 2018 21:51:38 -0300 James Almer wrote: > On 5/4/2018 9:19 PM, Michael Niedermayer wrote: > > On Fri, May 04, 2018 at 02:02:02PM -0300, James Almer wrote: > >> On 5/4/2018 1:51 PM, wm4 wrote: > >>> On Fri, 4 May 2018 13:30:38 -0300 > >>> James Almer wrote: > >>> > >>>> On 5/4/2018 12:58 PM, wm4 wrote: > >>>>> On Sat, 28 Apr 2018 19:05:29 +0200 > >>>>> wm4 wrote: > >>>>> > >>>>>> This can "demux" .vpy files. > >>>>>> > >>>>>> Some minor code copied from other LGPL parts of FFmpeg. > >>>>>> > >>>>>> I did not found a good way to test a few of the more obscure features, > >>>>>> like VFR nodes, compat pixel formats, or nodes with dynamic size/format > >>>>>> changes. These can be easily implemented on demand. > >>>>>> --- > >>>>>> configure | 5 + > >>>>>> libavformat/Makefile | 1 + > >>>>>> libavformat/allformats.c | 1 + > >>>>>> libavformat/vapoursynth.c | 421 > >>>>>> ++ > >>>>>> 4 files changed, 428 insertions(+) > >>>>>> create mode 100644 libavformat/vapoursynth.c > >>>>>> > >>>>> > >>>>> Pushed, with some minor changes, and zero-copy frame passing. > >>>> > >>>> The first step to fix something (in this case, usage sizeof(AVFrame) > >>>> outside libavutil) is not adding even more cases of the issue in > >>>> question. > >>>> You still can replace this with rawvideo. Someone even already wrote it > >>>> for you. > >>>> > >>>> Lets try to abide our own ABI rules... > >>> > >>> That's requires a frame copy and is not what I went through all the > >>> effort for. > >>> > >>> Why didn't you say anything when the kmsgrab code did the same thing? > >>> Or when the unwrapped frame stuff was added in the first place? > >> > >> I did the other day on IRC when you asked me why i was against this, if > >> you recall, because it was then when i found out this has been the case > >> for a long while, and why I'm now saying adding even more cases is going > >> in the opposite direction of an actual solution. > >> > >> In any case, i explicitly didn't block this, and no one else seems to > >> care, so whatever. > > > > I just now realized this, no, use of sizeof(AVFrame) outside libavutil is > > not ok, > > thats a ABI/API breakage. > > > > This must be removed/reverted. In all cases that are relevant. > > Fixed/adapted, not reverted. For vapoursynth and kmsgrab it should be a > matter of making them output rawvideo packets. Why don't you get it? We don't want to copy and then end up with unaligned shit. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Fri, 4 May 2018 13:30:38 -0300 James Almer wrote: > On 5/4/2018 12:58 PM, wm4 wrote: > > On Sat, 28 Apr 2018 19:05:29 +0200 > > wm4 wrote: > > > >> This can "demux" .vpy files. > >> > >> Some minor code copied from other LGPL parts of FFmpeg. > >> > >> I did not found a good way to test a few of the more obscure features, > >> like VFR nodes, compat pixel formats, or nodes with dynamic size/format > >> changes. These can be easily implemented on demand. > >> --- > >> configure | 5 + > >> libavformat/Makefile | 1 + > >> libavformat/allformats.c | 1 + > >> libavformat/vapoursynth.c | 421 > >> ++ > >> 4 files changed, 428 insertions(+) > >> create mode 100644 libavformat/vapoursynth.c > >> > > > > Pushed, with some minor changes, and zero-copy frame passing. > > The first step to fix something (in this case, usage sizeof(AVFrame) > outside libavutil) is not adding even more cases of the issue in question. > You still can replace this with rawvideo. Someone even already wrote it > for you. > > Lets try to abide our own ABI rules... That's requires a frame copy and is not what I went through all the effort for. Why didn't you say anything when the kmsgrab code did the same thing? Or when the unwrapped frame stuff was added in the first place? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Sat, 28 Apr 2018 19:05:29 +0200 wm4 wrote: > This can "demux" .vpy files. > > Some minor code copied from other LGPL parts of FFmpeg. > > I did not found a good way to test a few of the more obscure features, > like VFR nodes, compat pixel formats, or nodes with dynamic size/format > changes. These can be easily implemented on demand. > --- > configure | 5 + > libavformat/Makefile | 1 + > libavformat/allformats.c | 1 + > libavformat/vapoursynth.c | 421 > ++ > 4 files changed, 428 insertions(+) > create mode 100644 libavformat/vapoursynth.c > Pushed, with some minor changes, and zero-copy frame passing. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/isom: Recognize fourcc HEVC
On Fri, 4 May 2018 08:56:18 -0400 Compn wrote: > On Fri, 4 May 2018 14:36:03 +0200, wm4 wrote: > > > FourCC are already split up for avi and mov. Apparently it got > > sabotaged at some point. > > i think mkv re-uses riff list from avi on purpose, as it was meant as a > replacement to avi (and has all the vfw codec tags in the spec). > duplicating mkv and avi seems like a bad idea as they use the same tags. > > https://www.matroska.org/technical/guides/faq/index.html mkv has a VfW muxing mode, in which it uses AVI headers and FourCCs in the codec data. They're completely separate from normal Matroska codec IDs, which are strings. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/hls: don't propagate deprecated "user-agent" AVOption
On Sun, 29 Apr 2018 14:23:17 +0200
Clément Bœsch wrote:
> On Sat, Apr 28, 2018 at 08:37:06PM +0200, wm4 wrote:
> > This code will print a warning if any user agent is set - even if the
> > API user used the proper non-deprecated "user_agent" option.
> >
> > This change should not even break anything, because even if the user
> > sets the deprecated "user-agent" option, http.c copies it to the
> > "user_agent" option anyway.
> > ---
> > libavformat/hls.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index ffec124818..4ee4be769d 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -1593,7 +1593,7 @@ static int save_avio_options(AVFormatContext *s)
> > {
> > HLSContext *c = s->priv_data;
> > static const char * const opts[] = {
> > -"headers", "http_proxy", "user_agent", "user-agent", "cookies",
> > "referer", "rw_timeout", NULL };
> > +"headers", "http_proxy", "user_agent", "cookies", "referer",
> > "rw_timeout", NULL };
> > const char * const * opt = opts;
> > uint8_t *buf;
> > int ret = 0;
>
> Should be fine, thanks.
>
Pushed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: do not write timebase as framerate
On Sat, 28 Apr 2018 19:24:21 +0200 wm4 wrote: > If the API user doesn't set avg_frame_rate, matroskaenc will write the > current timebase as "default duration" for the video track. This makes > no sense, because the "default duration" implies the framerate of the > video. Since the timebase is forced to 1/1000, this will make the > resulting file claim 1000fps. > > Drop it and don't write the element. It's optional, so it's better not > to write it if the framerate is unknown. > > Strangely does not require FATE changes. > --- > libavformat/matroskaenc.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 5950b4de44..b7ff1950d3 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -1304,8 +1304,6 @@ static int mkv_write_track(AVFormatContext *s, > MatroskaMuxContext *mkv, > if( st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0 > && av_cmp_q(av_inv_q(st->avg_frame_rate), st->time_base) > 0) > put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 10LL > * st->avg_frame_rate.den / st->avg_frame_rate.num); > -else > -put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 10LL > * st->time_base.num / st->time_base.den); > > if (!native_id && > ff_codec_get_tag(ff_codec_movvideo_tags, par->codec_id) && Pushed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/isom: Recognize fourcc HEVC
On Fri, 4 May 2018 08:19:27 -0400 Compn wrote: > On Thu, 3 May 2018 17:26:38 -0300, James Almer > wrote: > > Create a new table with all non standard codecids from ff_codec_bmp_tags > > and use it exclusively in avidec, then remove them from > > ff_codec_bmp_tags, adapting other de/muxers if needed (like ivf). > > > > And then add this HEVC codecid to it. > > as fourcc maintainer i approve carls patch to add h265. I'm against it. > while i like james's idea, it would create a duplicated mess > of fourcc lists in each container. the fourcc problem got worse as > more people were using mov tags in avi, which is why the hack was > requested to have the avi container look at the isom list. saving us > from duplicating the isom list in the avi list. mostly due to apple > users needing an intermediate format for their apple codecs. > > i would be extremely against splitting and duplicating the > decoding fourccs again. internal and external fourccs like VLC and > mplayer would be the end game to that. ugh. VLC and mplayer need internal FourCCs because of their architecture. It's not relevant to FFmpeg. > > my idea would be to create a whitelist of codec ids we can encode to, > instead of splitting up the decode fourcc list amongst formats. FourCC are already split up for avi and mov. Apparently it got sabotaged at some point. A mux whitelist would be just as much as splitting the list. (Although having separate mux lists would be a good idea too.) Also here's an advice to FFmpeg: STOP ALLOWING USERS TO CREATE INVALID FILES. I'm sick of working them around. Stop creating a fucked up mess. It starts with rejecting this patch. > that way we fully control which fourcc we support in which codecs on > the encoder side (avienc, movenc). and we dont mess with the decoder > fourccs at all. > > please let me know if my idea would work, i am probably missing some > huge flaw somewhere... > > -compn > ___ > 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] flvenc: Fix sequence header update timestamps
On Fri, 4 May 2018 17:31:52 +0800 Steven Liu wrote: > > On 4 May 2018, at 17:20, Jan Ekström wrote: > > > > WHAT!? > > > > This LGTM was for the bit mask *only* > > > > ALL OTHER POINTS STAND > > Don’t worry , all LGTM too, this could fix the resend sequence header > timestamp problem, all should be ok. Well it doesn't LGT him. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavcodec/cuviddec A53CC closed captions support added to cuviddec & nvenc
On Thu, 3 May 2018 12:11:56 +0300 Andrey Turkin wrote: > cuvid decoder has one advantage over nvdec: it has a hardware deinterlacer > support. BTW not sure if this patch takes that into account. So cuvid is > the only way to get GPU-deinterlaced frames until someone makes CUDA-based > deinterlace filter. That's not an argument. Ask nvidia to expose their hw deinterlacer properly, or if it already is, ask them to document it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 12/24] avcodec/mjpegenc: add support for non-YUVJ formats
On Wed, 2 May 2018 19:37:27 +0100 Rostislav Pehlivanov wrote: > On 2 May 2018 at 19:16, wm4 wrote: > > > On Wed, 2 May 2018 19:14:21 +0100 > > Rostislav Pehlivanov wrote: > > > > > On 2 May 2018 at 19:04, Marton Balint wrote: > > > > > > > > > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > > > > > On 1 May 2018 at 23:22, Paul B Mahol wrote: > > > >> > > > >> On 5/2/18, Michael Niedermayer wrote: > > > >>> > Hi > > > >>> > > > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > > > >>> >> Signed-off-by: Paul B Mahol > > > >>> >> --- > > > >>> >> libavcodec/mjpegenc.c | 5 - > > > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > > > >>> >> tests/lavf-regression.sh | 2 +- > > > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > > > >>> > > > > >>> > This doesnt work > > > >>> > > > >>> It does work, if you want full range set full range as option. > > > >>> Check visually colors. > > > >>> This happens because rgb is still marked incorrectly as limited > > range, > > > >>> so limited is picked instead. > > > >>> > > > >>> > > > >> Shouldn't this be part of the patchset? > > > >> > > > > > > > > I thought that color_range only matters for YUV formats, for RGB it is > > > > ignored. If it was decided to support limited RGB, then please update > > the > > > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB > > format > > > > should not matter. > > > > > > > > On the other hand, if we decide to support limited RGB, then maybe a > > new > > > > member in color_range is better, this way we are able to specify more > > > > easily that we support limited YUV and full range RGB. (like ffplay > > > > typically does). > > > > > > > > > > Limited range RGB does NOT exist in the computer world, and I don't give > > a > > > fuck if broadcasting uses it. I've fought hard to keep it from getting > > into > > > AV1. > > > So no, we will not be supporting limited range RGB. > > > > You're very wrong. GPUs and HDMI (basically trash fire consumer > > garbage) sometimes seem to require you to feed limited RGB. > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > Maybe if you took a second to read what I wrote. HDMI sometimes requires > limited range rgb because its derived from broadcasting junk where they had > technical limitations they weren't smart enough to overcome. GPUs can > generate or convert RGB to limited range only for transmission, through the > use of gamma LUTs I believe. By that logic, aren't all mpeg style codecs (including vp9, av1, etc.) also "broadcasting junk"? I mean, even YUV could be full range if it weren't for broadcasting legacy. > Either way, limited range RGB does NOT exist in computers, as in, not > inside codecs, nor is there anything supporting such, and we will certainly > not start doing it. Denial of reality, I guess. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 12/24] avcodec/mjpegenc: add support for non-YUVJ formats
On Wed, 2 May 2018 19:14:21 +0100 Rostislav Pehlivanov wrote: > On 2 May 2018 at 19:04, Marton Balint wrote: > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > On 1 May 2018 at 23:22, Paul B Mahol wrote: > >> > >> On 5/2/18, Michael Niedermayer wrote: > >>> > Hi > >>> > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > >>> >> Signed-off-by: Paul B Mahol > >>> >> --- > >>> >> libavcodec/mjpegenc.c | 5 - > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > >>> >> tests/lavf-regression.sh | 2 +- > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > >>> > > >>> > This doesnt work > >>> > >>> It does work, if you want full range set full range as option. > >>> Check visually colors. > >>> This happens because rgb is still marked incorrectly as limited range, > >>> so limited is picked instead. > >>> > >>> > >> Shouldn't this be part of the patchset? > >> > > > > I thought that color_range only matters for YUV formats, for RGB it is > > ignored. If it was decided to support limited RGB, then please update the > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB format > > should not matter. > > > > On the other hand, if we decide to support limited RGB, then maybe a new > > member in color_range is better, this way we are able to specify more > > easily that we support limited YUV and full range RGB. (like ffplay > > typically does). > > > > Limited range RGB does NOT exist in the computer world, and I don't give a > fuck if broadcasting uses it. I've fought hard to keep it from getting into > AV1. > So no, we will not be supporting limited range RGB. You're very wrong. GPUs and HDMI (basically trash fire consumer garbage) sometimes seem to require you to feed limited RGB. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct
On Wed, 2 May 2018 17:49:05 +0100 Rostislav Pehlivanov wrote: > On 2 May 2018 at 17:39, wm4 wrote: > > > On Wed, 2 May 2018 13:44:48 +0200 > > Nicolas George wrote: > > > > > Vittorio Giovara (2018-05-01): > > > > Well no, let's step back a little. > > > > > > > > First of all there is no stigma about adding fields to AVCodecContext, > > > > in fact, the more, the merrier, right? > > > > > > > > Secondly, there _is_ concern about adding a field to AVCodec (not > > avctx), > > > > since is a delicate point of entry, and very sensitive to ABI (which > > in fact > > > > was broken in the first iteration of this patch). > > > > > > > > Finally, I immensely disagree that this is a good API at all, just > > throwing > > > > fields to structure is rarely a good approach to anything. On a > > separate note > > > > I also disagree that color_range (despite its importance) is an > > "essential > > > > property of images", at least not any more than the other color > > propeties: > > > > it is simply being noticed because there are special purpose enum > > values, but > > > > that doesn't mean that the other properties will work "just fine". > > Because > > > > they don't. > > > > The transfer characteristics for example is a much more impactful and > > > > delicate property that can affect the the color of the image in a much > > > > more profund way than color range. > > > > Similar arguments could be made for primaries and matrix. > > > > > > > > As much as you may hate hear it, this proposed filtering system does > > *not* > > > > belong to neither lavfi, lavf or lavc, but rather to the scaling > > library. > > > > Unfortunately the one used by default, swscale, is a monster spaghetti > > code > > > > and too difficult to work around. That however should not affect the > > design > > > > of APIs used in the other libraries: if swscale is not suited anymore > > for > > > > its job it should be replaced, you shouldn't be adding workarounds to > > AVCodec. > > > > > > Thanks for taking the time to express the reservations. I fully agree > > > with what you wrote. > > > > Please don't prevent us from fixing libavfilter. > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > +1 > > also what Vittorio complained about (AVCodec fields) has NOTHING to do with > lavfi or swscale. Well, it sort of strides all components (my fault), but surely we don't want to just stop solving this problem just because we have no "perfect" solution. Also this is the typical ffmpeg bikeshed brake: someone wants to solve a small problem, others bikeshed it to death because they really want to solve a much bigger problem that requires much more effort, and thus doesn't get solved. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct
On Wed, 2 May 2018 13:44:48 +0200 Nicolas George wrote: > Vittorio Giovara (2018-05-01): > > Well no, let's step back a little. > > > > First of all there is no stigma about adding fields to AVCodecContext, > > in fact, the more, the merrier, right? > > > > Secondly, there _is_ concern about adding a field to AVCodec (not avctx), > > since is a delicate point of entry, and very sensitive to ABI (which in fact > > was broken in the first iteration of this patch). > > > > Finally, I immensely disagree that this is a good API at all, just throwing > > fields to structure is rarely a good approach to anything. On a separate > > note > > I also disagree that color_range (despite its importance) is an "essential > > property of images", at least not any more than the other color propeties: > > it is simply being noticed because there are special purpose enum values, > > but > > that doesn't mean that the other properties will work "just fine". Because > > they don't. > > The transfer characteristics for example is a much more impactful and > > delicate property that can affect the the color of the image in a much > > more profund way than color range. > > Similar arguments could be made for primaries and matrix. > > > > As much as you may hate hear it, this proposed filtering system does *not* > > belong to neither lavfi, lavf or lavc, but rather to the scaling library. > > Unfortunately the one used by default, swscale, is a monster spaghetti code > > and too difficult to work around. That however should not affect the design > > of APIs used in the other libraries: if swscale is not suited anymore for > > its job it should be replaced, you shouldn't be adding workarounds to > > AVCodec. > > Thanks for taking the time to express the reservations. I fully agree > with what you wrote. Please don't prevent us from fixing libavfilter. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 01/24] avcodec: add color_range to AVCodec struct
On Tue, 1 May 2018 16:46:37 -0400
Vittorio Giovara wrote:
> --
>
> On 5/1/2018 4:39 PM, Paul B Mahol wrote:
> > Signed-off-by: Paul B Mahol
> > ---
> > libavcodec/avcodec.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index fb0c6fae70..3a8f69243c 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3433,6 +3433,7 @@ typedef struct AVCodec {
> > uint8_t max_lowres; ///< maximum value for lowres
> > supported by the decoder
> > const AVClass *priv_class; ///< AVClass for the private
> > context
> > const AVProfile *profiles; ///< array of recognized
> > profiles, or NULL if unknown, array is terminated by {FF_PROFILE_UNKNOWN}
> > +const enum AVColorRange *color_ranges; ///< array of supported color
> > ranges by encoder, or NULL if unknown, array is terminated by
> > AVCOL_RANGE_UNSPECIFIED
> >
> > /**
> > * Group name of the codec implementation.
> >
>
> While I appreciate this effort to remove the year-long deprecated J-pixel
> format, I have a feeling that this is not the right way to do it.
>
> I have no doubt that the code presented here works as expected, however
> adding YetAnotherField to the AVCodec structure is a slippery road. Namely
> only adding color range as an extra feature of the pixel format is incomplete.
>
> We should add every other single color parameter in this structure, insert
> the appropriate conversion filters (which swscale is not fully able to
> produce) and handle correct format negotiation across the chain, which is
> not exactly trivial (ie. which color space should be favoured? which one has
> a larger gamut? and so on).
>
> This creates a precedent that, I think, is bad API-wise and user-wise.
> I would rather keep the J-format around than creating a years long user-facing
> problem. Additionally having this field (or these fields if we are to include
> the other color properties) around makes the intrisic problem even harder
> to properly fix.
>
> I am aware that the goal of this patchset is not to properly support all
> color conversion properties, and it's just to being able to drop the
> J-formats,
> but I hope that this feedback will give a larger picture of the problem
> involved, that it will help you in making the right decision about this set.
I think the patch at hand is OK if it can finally get rid of the J
formats. To me as an API users these actually cause nothing but
annoyance, so it's really great to get rid of them.
Sure, this doesn't solve negotiating of video attributes in
libavfilter, or accurately exposing all encoder capabilities. There are
plenty of other such cases (ask if you want examples). Solving this in
a general and complete way is _hard_. It's not just about adding
missing fields. On a side note, we should think of something that
doesn't require us duplicating all these metadata fields in AVCodec,
AVCodecContext, AVFrame, avfilter buffer sink, avfilter buffer src, and
having to touch every single to "negotiate" the attribute.
So just adding support for color range for the sole purpose of removing
the J formats sounds good to me.
What do you suggest?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: Fallback to duration if sample rate is unavailable
On Tue, 1 May 2018 22:44:07 +0200
Michael Niedermayer wrote:
> Regression since: af1761f7
> Fixes: Division by 0
> Fixes: ffmpeg_crash_1
>
> Found-by: Thuan Pham, Marcel Böhme, Andrew Santosa and Alexandru Razvan
> Caciulescu with AFLSmart
> Signed-off-by: Michael Niedermayer
> ---
> fftools/ffmpeg.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 5dc198f933..15fa54f24e 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2706,8 +2706,12 @@ static int process_input_packet(InputStream *ist,
> const AVPacket *pkt, int no_eo
> ist->dts = ist->next_dts;
> switch (ist->dec_ctx->codec_type) {
> case AVMEDIA_TYPE_AUDIO:
> -ist->next_dts += ((int64_t)AV_TIME_BASE *
> ist->dec_ctx->frame_size) /
> - ist->dec_ctx->sample_rate;
> +if (ist->dec_ctx->sample_rate) {
> +ist->next_dts += ((int64_t)AV_TIME_BASE *
> ist->dec_ctx->frame_size) /
> + ist->dec_ctx->sample_rate;
> +} else {
> +ist->next_dts += av_rescale_q(pkt->duration,
> ist->st->time_base, AV_TIME_BASE_Q);
> +}
> break;
> case AVMEDIA_TYPE_VIDEO:
> if (ist->framerate.num) {
Wouldn't it be better to error out here, instead of using yet another
unreliable value and "hoping for the best"?
Also I'm not sure, but I think unknown durations are sometimes set to
-1 instead of 0.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/yuv4mpegdec: simplify math
On Tue, 1 May 2018 17:48:32 +0200 Paul B Mahol wrote: > This one actually works with hd1080 y4m files when seeking backwards. > > Signed-off-by: Paul B Mahol > --- > libavformat/yuv4mpegdec.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c > index 8662a42a4c..de3d5637fe 100644 > --- a/libavformat/yuv4mpegdec.c > +++ b/libavformat/yuv4mpegdec.c > @@ -317,11 +317,9 @@ static int yuv4_read_seek(AVFormatContext *s, int > stream_index, > AVStream *st = s->streams[0]; > int64_t pos; > > -pos = av_rescale_rnd(pts * s->packet_size, > - st->time_base.num, > - st->time_base.den * s->packet_size, > - (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN : > AV_ROUND_UP); > -pos *= s->packet_size; > +if (flags & AVSEEK_FLAG_BACKWARD) > +pts -= 1; > +pos = pts * s->packet_size; > > if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0) > return -1; That seems questionable too. Though I'm not sure if the previous code is correct. In theory, you're supposed to do this: 1. Determine seek target as accurately as possible 2. If the seek target is not exact then: 2a. if AVSEEK_FLAG_FORWARD is set, then round to the next frame 2b. otherwise, round to the previous frame But if pts is in the stream timebase, then 2. can never happen. (And also, I have no idea what the code removed by this patch does.) Regarding mpv, can you just send me a reproducible test case? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/yuv4mpegdec: use generic code for seeking
On Tue, 1 May 2018 15:17:53 +0200
Paul B Mahol wrote:
> On 5/1/18, wm4 wrote:
> > On Tue, 1 May 2018 11:47:04 +0200
> > Paul B Mahol wrote:
> >
> >> Signed-off-by: Paul B Mahol
> >> ---
> >> libavformat/yuv4mpegdec.c| 23
> >> tests/ref/seek/lavf-yuv4mpeg | 52
> >>
> >> 2 files changed, 28 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> >> index 8662a42a4c..cf6da2a2ef 100644
> >> --- a/libavformat/yuv4mpegdec.c
> >> +++ b/libavformat/yuv4mpegdec.c
> >> @@ -306,28 +306,13 @@ static int yuv4_read_packet(AVFormatContext *s,
> >> AVPacket *pkt)
> >> return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
> >> }
> >> pkt->stream_index = 0;
> >> -pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> >> +pkt->pos = off;
> >> +pkt->dts = pkt->pts = (off - s->internal->data_offset) /
> >> s->packet_size;
> >> +pkt->flags |= AV_PKT_FLAG_KEY;
> >> pkt->duration = 1;
> >> return 0;
> >> }
> >>
> >> -static int yuv4_read_seek(AVFormatContext *s, int stream_index,
> >> - int64_t pts, int flags)
> >> -{
> >> -AVStream *st = s->streams[0];
> >> -int64_t pos;
> >> -
> >> -pos = av_rescale_rnd(pts * s->packet_size,
> >> - st->time_base.num,
> >> - st->time_base.den * s->packet_size,
> >> - (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN :
> >> AV_ROUND_UP);
> >> -pos *= s->packet_size;
> >> -
> >> -if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
> >> -return -1;
> >> -return 0;
> >> -}
> >> -
> >> static int yuv4_probe(AVProbeData *pd)
> >> {
> >> /* check file header */
> >> @@ -343,6 +328,6 @@ AVInputFormat ff_yuv4mpegpipe_demuxer = {
> >> .read_probe = yuv4_probe,
> >> .read_header= yuv4_read_header,
> >> .read_packet= yuv4_read_packet,
> >> -.read_seek = yuv4_read_seek,
> >> .extensions = "y4m",
> >> +.flags = AVFMT_GENERIC_INDEX,
> >> };
> >> diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg
> >
> > Seems like a bad idea. This will make seeking read and skip all data
> > when seeking to the middle of the stream, instead of just seeking there.
>
> How do you explain that this patch makes backsteeping with mpv magnitude
> faster?
>
> Without it, more far you are from start of file backstepping is linearly
> slower.
I don't know how this interacts with mpv, but obviously it's unexpected
that O(1) demuxer level seeking is slower than O(n) seeking. You
probably have some sort of overflow or something in the seek routine
or elsewhere. Maybe it's an lavf internal fallback to something worse
if the seek fails (e.g. when trying to seek before the start of the
file). Obviously you should analyze this.
(mpv seeks before the start of the file because it can't reliable know
a pts is before the start.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/yuv4mpegdec: use generic code for seeking
On Tue, 1 May 2018 11:47:04 +0200
Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol
> ---
> libavformat/yuv4mpegdec.c| 23
> tests/ref/seek/lavf-yuv4mpeg | 52
>
> 2 files changed, 28 insertions(+), 47 deletions(-)
>
> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> index 8662a42a4c..cf6da2a2ef 100644
> --- a/libavformat/yuv4mpegdec.c
> +++ b/libavformat/yuv4mpegdec.c
> @@ -306,28 +306,13 @@ static int yuv4_read_packet(AVFormatContext *s,
> AVPacket *pkt)
> return s->pb->eof_reached ? AVERROR_EOF : AVERROR(EIO);
> }
> pkt->stream_index = 0;
> -pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> +pkt->pos = off;
> +pkt->dts = pkt->pts = (off - s->internal->data_offset) / s->packet_size;
> +pkt->flags |= AV_PKT_FLAG_KEY;
> pkt->duration = 1;
> return 0;
> }
>
> -static int yuv4_read_seek(AVFormatContext *s, int stream_index,
> - int64_t pts, int flags)
> -{
> -AVStream *st = s->streams[0];
> -int64_t pos;
> -
> -pos = av_rescale_rnd(pts * s->packet_size,
> - st->time_base.num,
> - st->time_base.den * s->packet_size,
> - (flags & AVSEEK_FLAG_BACKWARD) ? AV_ROUND_DOWN :
> AV_ROUND_UP);
> -pos *= s->packet_size;
> -
> -if (avio_seek(s->pb, pos + s->internal->data_offset, SEEK_SET) < 0)
> -return -1;
> -return 0;
> -}
> -
> static int yuv4_probe(AVProbeData *pd)
> {
> /* check file header */
> @@ -343,6 +328,6 @@ AVInputFormat ff_yuv4mpegpipe_demuxer = {
> .read_probe = yuv4_probe,
> .read_header= yuv4_read_header,
> .read_packet= yuv4_read_packet,
> -.read_seek = yuv4_read_seek,
> .extensions = "y4m",
> +.flags = AVFMT_GENERIC_INDEX,
> };
> diff --git a/tests/ref/seek/lavf-yuv4mpeg b/tests/ref/seek/lavf-yuv4mpeg
Seems like a bad idea. This will make seeking read and skip all data
when seeking to the middle of the stream, instead of just seeking there.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: fix mixed code and declarations
On Sat, 28 Apr 2018 23:21:27 -0300
James Almer wrote:
> Signed-off-by: James Almer
> ---
> fftools/ffmpeg.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index d3054092ba..9b3e9d121e 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -4727,8 +4727,7 @@ static int transcode(void)
>
> static BenchmarkTimeStamps get_benchmark_time_stamps(void)
> {
> -BenchmarkTimeStamps time_stamps;
> -time_stamps.real_usec = av_gettime_relative();
> +BenchmarkTimeStamps time_stamps = { .real_usec = av_gettime_relative() };
> #if HAVE_GETRUSAGE
> struct rusage rusage;
>
> @@ -4833,10 +4832,11 @@ int main(int argc, char **argv)
> if (transcode() < 0)
> exit_program(1);
> if (do_benchmark) {
> +int64_t utime, stime, rtime;
> current_time = get_benchmark_time_stamps();
> -int64_t utime = current_time.user_usec - ti.user_usec;
> -int64_t stime = current_time.sys_usec - ti.sys_usec;
> -int64_t rtime = current_time.real_usec - ti.real_usec;
> +utime = current_time.user_usec - ti.user_usec;
> +stime = current_time.sys_usec - ti.sys_usec;
> +rtime = current_time.real_usec - ti.real_usec;
> av_log(NULL, AV_LOG_INFO,
> "bench: utime=%0.3fs stime=%0.3fs rtime=%0.3fs\n",
> utime / 100.0, stime / 100.0, rtime / 100.0);
It's pretty funny that we change something because it's not allowed in
C89 (but in C99), and add use of a C99 only feature in the same fix.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: do not write timebase as framerate
On Sat, 28 Apr 2018 21:18:47 +0200 Carl Eugen Hoyos wrote: > 2018-04-28 20:05 GMT+02:00, wm4 : > > On Sat, 28 Apr 2018 19:52:38 +0200 > > Carl Eugen Hoyos wrote: > > > >> 2018-04-28 19:24 GMT+02:00, wm4 : > >> > If the API user doesn't set avg_frame_rate, matroskaenc will write the > >> > current timebase as "default duration" for the video track. This makes > >> > no sense, because the "default duration" implies the framerate of the > >> > video. Since the timebase is forced to 1/1000, this will make the > >> > resulting file claim 1000fps. > >> > > >> > Drop it and don't write the element. It's optional, so it's better not > >> > to write it if the framerate is unknown. > >> > >> (Isn't it default frame duration?) > > > > The Matroska "spec" calls it DefaultDuration. > > Which sounds more similar to "default frame duration" than > "framerate"... It's the same thing, really. Technically the framerate is the inverse of it. > >> Please mention ticket #6386 if you commit. > > > > So if this was known to you, and you even made a patch, why > > did you never send the patch to the list? > > I am not convinced the patch is correct and the OP claimed > that it did not fix the reported issue completely. > > (And this was of course not known "to me" but to everybody.) It definitely does fix the issue that ffmpeg writes files that mkv demuxers, including ffmpeg's, detect as having video framerate of 1000hz. I don't know what that issue is about and I don't care, it's just that you posted a patch there that does exactly the same thing as mine. I'll gladly leave that to you. Will push tomorrow. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Sat, 28 Apr 2018 15:28:13 -0300 James Almer wrote: > On 4/28/2018 2:05 PM, wm4 wrote: > > This can "demux" .vpy files. > > > > +pkt->data = pkt->buf->data; > > +pkt->size = pkt->buf->size; > > +pkt->flags |= AV_PKT_FLAG_TRUSTED; > > + > > +if (vs->is_cfr) > > +pkt->pts = vs->current_frame; > > + > > +vs->current_frame++; > > + > > +end: > > +if (err < 0) > > +av_packet_unref(pkt); > > Unneeded. Nothing after pkt->buf is initialized can fail right now. And > if av_buffer_create() fails, pkt will be untouched. Could be good for robustness reasons, but OK, removed. > > +av_frame_free(&frame); > > +vs->vsapi->freeFrame(vsframe); > > +return err; > > Shouldn't you set err to pkt->size right above the end label? I'm not sure what you mean. err is initialized to 0, and only becomes non-0 on errors, so it doesn't need to be set here. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/hls: don't propagate deprecated "user-agent" AVOption
This code will print a warning if any user agent is set - even if the
API user used the proper non-deprecated "user_agent" option.
This change should not even break anything, because even if the user
sets the deprecated "user-agent" option, http.c copies it to the
"user_agent" option anyway.
---
libavformat/hls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/hls.c b/libavformat/hls.c
index ffec124818..4ee4be769d 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1593,7 +1593,7 @@ static int save_avio_options(AVFormatContext *s)
{
HLSContext *c = s->priv_data;
static const char * const opts[] = {
-"headers", "http_proxy", "user_agent", "user-agent", "cookies",
"referer", "rw_timeout", NULL };
+"headers", "http_proxy", "user_agent", "cookies", "referer",
"rw_timeout", NULL };
const char * const * opt = opts;
uint8_t *buf;
int ret = 0;
--
2.16.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: do not write timebase as framerate
On Sat, 28 Apr 2018 19:52:38 +0200 Carl Eugen Hoyos wrote: > 2018-04-28 19:24 GMT+02:00, wm4 : > > If the API user doesn't set avg_frame_rate, matroskaenc will write the > > current timebase as "default duration" for the video track. This makes > > no sense, because the "default duration" implies the framerate of the > > video. Since the timebase is forced to 1/1000, this will make the > > resulting file claim 1000fps. > > > > Drop it and don't write the element. It's optional, so it's better not > > to write it if the framerate is unknown. > > (Isn't it default frame duration?) The Matroska "spec" calls it DefaultDuration. > Please mention ticket #6386 if you commit. So if this was known to you, and you even made a patch, why did you never send the patch to the list? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat/matroskaenc: do not write timebase as framerate
If the API user doesn't set avg_frame_rate, matroskaenc will write the current timebase as "default duration" for the video track. This makes no sense, because the "default duration" implies the framerate of the video. Since the timebase is forced to 1/1000, this will make the resulting file claim 1000fps. Drop it and don't write the element. It's optional, so it's better not to write it if the framerate is unknown. Strangely does not require FATE changes. --- libavformat/matroskaenc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 5950b4de44..b7ff1950d3 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -1304,8 +1304,6 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv, if( st->avg_frame_rate.num > 0 && st->avg_frame_rate.den > 0 && av_cmp_q(av_inv_q(st->avg_frame_rate), st->time_base) > 0) put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 10LL * st->avg_frame_rate.den / st->avg_frame_rate.num); -else -put_ebml_uint(pb, MATROSKA_ID_TRACKDEFAULTDURATION, 10LL * st->time_base.num / st->time_base.den); if (!native_id && ff_codec_get_tag(ff_codec_movvideo_tags, par->codec_id) && -- 2.16.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
This can "demux" .vpy files.
Some minor code copied from other LGPL parts of FFmpeg.
I did not found a good way to test a few of the more obscure features,
like VFR nodes, compat pixel formats, or nodes with dynamic size/format
changes. These can be easily implemented on demand.
---
configure | 5 +
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/vapoursynth.c | 421 ++
4 files changed, 428 insertions(+)
create mode 100644 libavformat/vapoursynth.c
diff --git a/configure b/configure
index 9fa1665496..fcc93ecc30 100755
--- a/configure
+++ b/configure
@@ -303,6 +303,7 @@ External library support:
--disable-sdl2 disable sdl2 [autodetect]
--disable-securetransport disable Secure Transport, needed for TLS support
on OSX if openssl and gnutls are not used
[autodetect]
+ --enable-vapoursynth enable VapourSynth demuxer [no]
--disable-xlib disable xlib [autodetect]
--disable-zlib disable zlib [autodetect]
@@ -1724,6 +1725,7 @@ EXTERNAL_LIBRARY_LIST="
mediacodec
openal
opengl
+vapoursynth
"
HWACCEL_AUTODETECT_LIBRARY_LIST="
@@ -3088,6 +3090,7 @@ libx265_encoder_deps="libx265"
libxavs_encoder_deps="libxavs"
libxvid_encoder_deps="libxvid"
libzvbi_teletext_decoder_deps="libzvbi"
+vapoursynth_demuxer_deps="vapoursynth"
videotoolbox_suggest="coreservices"
videotoolbox_deps="corefoundation coremedia corevideo"
videotoolbox_encoder_deps="videotoolbox
VTCompressionSessionPrepareToEncodeFrames"
@@ -6130,6 +6133,8 @@ enabled rkmpp && { require_pkg_config rkmpp
rockchip_mpp rockchip/r
{ enabled libdrm ||
die "ERROR: rkmpp requires --enable-libdrm"; }
}
+enabled vapoursynth && require_pkg_config vapoursynth
"vapoursynth-script >= 42" VSScript.h vsscript_init
+
if enabled gcrypt; then
GCRYPT_CONFIG="${cross_prefix}libgcrypt-config"
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 3eeca5091d..e1e74a8f43 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -570,6 +570,7 @@ OBJS-$(CONFIG_LIBRTMPTE_PROTOCOL)+= librtmp.o
OBJS-$(CONFIG_LIBSRT_PROTOCOL) += libsrt.o
OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
+OBJS-$(CONFIG_VAPOURSYNTH_DEMUXER) += vapoursynth.o
# protocols I/O
OBJS-$(CONFIG_ASYNC_PROTOCOL)+= async.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index d582778b3b..a94364f41d 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -482,6 +482,7 @@ extern AVOutputFormat ff_chromaprint_muxer;
extern AVInputFormat ff_libgme_demuxer;
extern AVInputFormat ff_libmodplug_demuxer;
extern AVInputFormat ff_libopenmpt_demuxer;
+extern AVInputFormat ff_vapoursynth_demuxer;
#include "libavformat/muxer_list.c"
#include "libavformat/demuxer_list.c"
diff --git a/libavformat/vapoursynth.c b/libavformat/vapoursynth.c
new file mode 100644
index 00..6758a7bf31
--- /dev/null
+++ b/libavformat/vapoursynth.c
@@ -0,0 +1,421 @@
+/*
+ * 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
+ */
+
+/**
+* @file
+* VapourSynth demuxer
+*
+* Synthesizes vapour (?)
+*/
+
+#include
+
+#include
+#include
+
+#include "libavutil/avassert.h"
+#include "libavutil/avstring.h"
+#include "libavutil/eval.h"
+#include "libavutil/imgutils.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+#include "avformat.h"
+#include "internal.h"
+
+typedef struct VSContext {
+const AVClass *class;
+
+const VSAPI *vsapi;
+VSCore *vscore;
+VSScript *vss;
+int did_init;
+
+VSNodeRef *outnode;
+int is_cfr;
+int current_frame;
+
+int c_order[4];
+
+/* options */
+int64_t max_size;
+} VSContext;
+
+#define OFFSET(x) offsetof(VSContext, x)
+#define A AV_OPT_FLAG_AUDIO_PARAM
+#define D AV_OPT_FLAG_DECODING_PARAM
+static const AVOption options[] = {
+{"max_size","set max file size supported (in bytes)",
OFFSET(max_size),AV_OPT_TYPE_INT64, {.i64 = 1 * 1024 * 1024}, 0,
SIZE_MAX - 1, A|D},
+{NULL
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Sat, 28 Apr 2018 11:58:27 -0300 James Almer wrote: > On 4/28/2018 8:51 AM, wm4 wrote: > > On Sat, 28 Apr 2018 11:08:01 +0800 > > Steven Liu wrote: > > > >>> On 28 Apr 2018, at 03:37, wm4 wrote: > >>> > > > >>> + > >>> +if (strncmp(pd->name, "xyz", 3) == 0) > >>> +continue; > >> > >> liuqideMacBook-Pro:xxx liuqi$ ../tools/patcheck > >> ~/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch > >> patCHeck 1e10.0 > >> This tool is intended to help a human check/review patches. It is very far > >> from > >> being free of false positives and negatives, and its output are just hints > >> of what > >> may or may not be bad. When you use it and it misses something or detects > >> something wrong, fix it and send a patch to the ffmpeg-devel mailing list. > >> License: GPL, Author: Michael Niedermayer > >> egrep: empty (sub)expression > >> > >> These functions may need av_cold, please review the whole patch for > >> similar functions needing av_cold > >> /Users/liuqi/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch:147:+static > >> int is_native_endian(enum AVPixelFormat pixfmt) > > > > av_cold seems dumb but I guess I can cargo-cult that. > > Unless av_cold is needed/expected for AVInputFormat->read_header like > it's done for AVCodec->init, none of these functions need it at all. > Especially since they will be inlined by the compiler anyway. Only a small number of functions/formats in libavformat use av_cold. But it doesn't harm either. > At most, make the small ones like is_native_endian explicitly inline. Doesn't seem needed either. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Fri, 27 Apr 2018 22:27:47 -0300 James Almer wrote: > On 4/27/2018 4:37 PM, wm4 wrote: > > From: wm4 > > > > require_pkg_config libvapoursynth "vapoursynth-script >= 42" VSScript.h > > vsscript_init || die "ERROR: vapoursynth or vsscript not found"; > > die() is needed only with test_pkg_config and check_pkg_config, not > require_pkg_config. OK, will remove. > And seeing that vapoursynth-script.pc depends on vapoursynth.pc, you can > simplify all this by only checking for vapoursynth-script. I'm not sure if I should rely on this. But considering VSScript.h includes VapourSynth.h anyway, and we don't use any VapourSynth functions directly, the risk is low, so I'll remove it. > > +st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; > > +st->codecpar->codec_id = AV_CODEC_ID_WRAPPED_AVFRAME; > > What exactly is the use case for this? You mean using wrapped avframe? It means you don't have to mess with raw encoding/decoding. Unless we have some fancy way to make rawdec.c accept frames packed by av_image_copy_to_buffer() without having to map pixfmts to raw codecs? (rawdec.c is a lot of code, I might have missed a simple way to make it accept that.) > > +pkt->buf = av_buffer_create((uint8_t*)frame, sizeof(*frame), > > So, the whole wrapped avframe is pretty much fucked up since conception. > Did nobody notice that lavc/wrapped_avframe.c is using sizeof(AVFrame)? > > Much like in here, it's wrong and Bad Things(tm) will happen as soon as > we add a new field. Ah, that is true. In theory it's in conflict with out policy that libavutil can append fields to AVFrame, without rebuilding libavcodec. I've copied this AVFrame packing code from kmsgrab.c though (which is also new code, and which I've noticed leaks memory when handling malloc failure). I think it'd be best if we had updated "non-dumb" side data which can manage AVFrames in a correct way. But it'll be a long way until this happens, apparently. If you really want to solve this, we could introduce an av_frame_get_struct_size() call, which would be dumb, but correct. (Or maybe make it avpriv_?) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
On Sat, 28 Apr 2018 11:08:01 +0800 Steven Liu wrote: > > On 28 Apr 2018, at 03:37, wm4 wrote: > > > > + > > +if (strncmp(pd->name, "xyz", 3) == 0) > > +continue; > > liuqideMacBook-Pro:xxx liuqi$ ../tools/patcheck > ~/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch > patCHeck 1e10.0 > This tool is intended to help a human check/review patches. It is very far > from > being free of false positives and negatives, and its output are just hints of > what > may or may not be bad. When you use it and it misses something or detects > something wrong, fix it and send a patch to the ffmpeg-devel mailing list. > License: GPL, Author: Michael Niedermayer > egrep: empty (sub)expression > > These functions may need av_cold, please review the whole patch for similar > functions needing av_cold > /Users/liuqi/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch:147:+static > int is_native_endian(enum AVPixelFormat pixfmt) av_cold seems dumb but I guess I can cargo-cult that. > x==0 / x!=0 can be simplified to !x / x > /Users/liuqi/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch:193:+ > if (strncmp(pd->name, "xyz", 3) == 0) > /Users/liuqi/Downloads/FFmpeg-devel-avformat-add-vapoursynth-wrapper.patch:204:+ > c->offset != 0 || c->shift != 0 || I prefer to keep those explicit, especially the strncmp one. But I don't have that strong feelings about it, so if someone minds I can still change it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat: add vapoursynth wrapper
From: wm4
This can "demux" .vpy files.
Some minor code copied from other LGPL parts of FFmpeg.
Possibly support VS compat pixel formats.
TODO:
- check whether VS can change format midstream
- test vfr mode, return proper timestamps when using it
- drop "lib" prefix?
---
configure| 4 +
libavformat/Makefile | 1 +
libavformat/allformats.c | 1 +
libavformat/libvapoursynth.c | 379 +++
4 files changed, 385 insertions(+)
create mode 100644 libavformat/libvapoursynth.c
diff --git a/configure b/configure
index 9fa1665496..17e46c5daa 100755
--- a/configure
+++ b/configure
@@ -265,6 +265,7 @@ External library support:
if openssl or gnutls is not used [no]
--enable-libtwolame enable MP2 encoding via libtwolame [no]
--enable-libv4l2 enable libv4l2/v4l-utils [no]
+ --enable-libvapoursynth enable VapourSynth demuxer [no]
--enable-libvidstab enable video stabilization using vid.stab [no]
--enable-libvmaf enable vmaf filter via libvmaf [no]
--enable-libvo-amrwbenc enable AMR-WB encoding via libvo-amrwbenc [no]
@@ -1712,6 +1713,7 @@ EXTERNAL_LIBRARY_LIST="
libtheora
libtwolame
libv4l2
+libvapoursynth
libvorbis
libvpx
libwavpack
@@ -3068,6 +3070,7 @@ libspeex_encoder_deps="libspeex"
libspeex_encoder_select="audio_frame_queue"
libtheora_encoder_deps="libtheora"
libtwolame_encoder_deps="libtwolame"
+libvapoursynth_demuxer_deps="libvapoursynth"
libvo_amrwbenc_encoder_deps="libvo_amrwbenc"
libvorbis_decoder_deps="libvorbis"
libvorbis_encoder_deps="libvorbis libvorbisenc"
@@ -6041,6 +6044,7 @@ enabled libtwolame&& require libtwolame twolame.h
twolame_init -ltwolame
{ check_lib libtwolame twolame.h
twolame_encode_buffer_float32_interleaved -ltwolame ||
die "ERROR: libtwolame must be installed and
version must be >= 0.3.10"; }
enabled libv4l2 && require_pkg_config libv4l2 libv4l2 libv4l2.h
v4l2_ioctl
+enabled libvapoursynth&& require_pkg_config libvapoursynth "vapoursynth >=
42" VapourSynth.h getVapourSynthAPI && require_pkg_config libvapoursynth
"vapoursynth-script >= 42" VSScript.h vsscript_init || die "ERROR: vapoursynth
or vsscript not found";
enabled libvidstab&& require_pkg_config libvidstab "vidstab >= 0.98"
vid.stab/libvidstab.h vsMotionDetectInit
enabled libvmaf && require_pkg_config libvmaf "libvmaf >= 0.6.2"
libvmaf.h compute_vmaf
enabled libvo_amrwbenc&& require libvo_amrwbenc vo-amrwbenc/enc_if.h
E_IF_init -lvo-amrwbenc
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 3eeca5091d..731b7ac714 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -570,6 +570,7 @@ OBJS-$(CONFIG_LIBRTMPTE_PROTOCOL)+= librtmp.o
OBJS-$(CONFIG_LIBSRT_PROTOCOL) += libsrt.o
OBJS-$(CONFIG_LIBSSH_PROTOCOL) += libssh.o
OBJS-$(CONFIG_LIBSMBCLIENT_PROTOCOL) += libsmbclient.o
+OBJS-$(CONFIG_LIBVAPOURSYNTH_DEMUXER)+= libvapoursynth.o
# protocols I/O
OBJS-$(CONFIG_ASYNC_PROTOCOL)+= async.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index d582778b3b..67f6c4339c 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -482,6 +482,7 @@ extern AVOutputFormat ff_chromaprint_muxer;
extern AVInputFormat ff_libgme_demuxer;
extern AVInputFormat ff_libmodplug_demuxer;
extern AVInputFormat ff_libopenmpt_demuxer;
+extern AVInputFormat ff_libvapoursynth_demuxer;
#include "libavformat/muxer_list.c"
#include "libavformat/demuxer_list.c"
diff --git a/libavformat/libvapoursynth.c b/libavformat/libvapoursynth.c
new file mode 100644
index 00..95699e81d2
--- /dev/null
+++ b/libavformat/libvapoursynth.c
@@ -0,0 +1,379 @@
+/*
+ * 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
+ */
+
+/**
+* @file
+* VapourSynth demuxer
+*
+* Synthesizes vapour (?)
+*/
+
+#include
+
Re: [FFmpeg-devel] github
On Thu, 26 Apr 2018 14:41:55 +0200 Daniel Oberhoff wrote: > > On 26. Apr 2018, at 14:40, wm4 wrote: > > > > On Thu, 26 Apr 2018 14:12:14 +0200 > > Hendrik Leppkes mailto:h.lepp...@gmail.com>> wrote: > > > >> On Thu, Apr 26, 2018 at 2:02 PM, Daniel Oberhoff > >> wrote: > >>> > >>>> Am 26.04.2018 um 13:59 schrieb Daniel Oberhoff > >>>> : > >>>> > >>>> > >>>>> Am 26.04.2018 um 13:56 schrieb Daniel Oberhoff > >>>>> : > >>>>> > >>>>> > >>>>>> Am 26.04.2018 um 13:52 schrieb Nicolas George : > >>>>>> > >>>>>> Daniel Oberhoff (2018-04-26): > >>>>>>> I was wondering if there is any chance to move development to github? > >>>>>>> I.e. not just mirror, but as primary development repo, with issues and > >>>>>>> pull requests? Would make collaboration a *lot* easier (think of > >>>>>>> submitting a pr instead of having to generate/format/split patches). > >>>>>> > >>>>>> If development involves working in a web browser a lot, count me out. > >>>>>> Can you point me to the command-line > >>>>> > >>>>> https://hub.github.com/hub.1.html > >>>> > >>>> But you can’t really do reviews that way, so the criticism stands. > >>> > >>> BTW, is there any kind of issue tracking? > >> > >> https://trac.ffmpeg.org/ > > > > To be fair, I'd prefer the github issue tracker over TRAC any day. > > Still has the other problems I mentioned. > > gitlab? > That would mostly get rid of the centralization argument. But I've heard bad things from someone who wanted to setup a private instance of it. Apparently it has a large number of dependencies, is extremely hard to deploy (unless you use their docker container), and it's SLOW. In fact even gitlab.com seems to have severe performance problems occasionally. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add system and real time to benchmarking.
On Fri, 20 Apr 2018 16:21:13 -0400
Mark Wachsler wrote:
> The -benchmark and -benchmark_all options now show user, system, and real
> time,
> instead of just user time.
> ---
> fftools/ffmpeg.c | 50 ++--
> 1 file changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 4dbe72186d..d37171d567 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -120,8 +120,14 @@ const char *const forced_keyframes_const_names[] = {
> NULL
> };
>
> +typedef struct TimeStamps {
> + int64_t real_usec;
> + int64_t user_usec;
> + int64_t sys_usec;
> +} TimeStamps;
> +
> static void do_video_stats(OutputStream *ost, int frame_size);
> -static int64_t getutime(void);
> +static TimeStamps get_time_stamps(void);
> static int64_t getmaxrss(void);
> static int ifilter_has_all_input_formats(FilterGraph *fg);
>
> @@ -133,7 +139,7 @@ static int64_t decode_error_stat[2];
>
> static int want_sdp = 1;
>
> -static int current_time;
> +static TimeStamps current_time;
> AVIOContext *progress_avio = NULL;
>
> static uint8_t *subtitle_out;
> @@ -653,7 +659,7 @@ static void abort_codec_experimental(AVCodec *c, int
> encoder)
> static void update_benchmark(const char *fmt, ...)
> {
> if (do_benchmark_all) {
> -int64_t t = getutime();
> +TimeStamps t = get_time_stamps();
> va_list va;
> char buf[1024];
>
> @@ -661,7 +667,11 @@ static void update_benchmark(const char *fmt, ...)
> va_start(va, fmt);
> vsnprintf(buf, sizeof(buf), fmt, va);
> va_end(va);
> -av_log(NULL, AV_LOG_INFO, "bench: %8"PRIu64" %s \n", t -
> current_time, buf);
> +av_log(NULL, AV_LOG_INFO,
> + "bench: %8" PRIu64 " user %8" PRIu64 " sys %8" PRIu64 "
> real %s \n",
> + t.user_usec - current_time.user_usec,
> + t.sys_usec - current_time.sys_usec,
> + t.real_usec - current_time.real_usec, buf);
> }
> current_time = t;
> }
> @@ -4715,23 +4725,30 @@ static int transcode(void)
> return ret;
> }
>
> -
> -static int64_t getutime(void)
> -{
> +static TimeStamps get_time_stamps(void) {
> + TimeStamps time_stamps;
> + time_stamps.real_usec = av_gettime_relative();
> #if HAVE_GETRUSAGE
> struct rusage rusage;
>
> getrusage(RUSAGE_SELF, &rusage);
> -return (rusage.ru_utime.tv_sec * 100LL) + rusage.ru_utime.tv_usec;
> +time_stamps.user_usec =
> +(rusage.ru_utime.tv_sec * 100LL) + rusage.ru_utime.tv_usec;
> +time_stamps.sys_usec =
> +(rusage.ru_stime.tv_sec * 100LL) + rusage.ru_stime.tv_usec;
> #elif HAVE_GETPROCESSTIMES
> HANDLE proc;
> FILETIME c, e, k, u;
> proc = GetCurrentProcess();
> GetProcessTimes(proc, &c, &e, &k, &u);
> -return ((int64_t) u.dwHighDateTime << 32 | u.dwLowDateTime) / 10;
> +time_stamps.user_usec =
> +((int64_t)u.dwHighDateTime << 32 | u.dwLowDateTime) / 10;
> +time_stamps.sys_usec =
> +((int64_t)k.dwHighDateTime << 32 | k.dwLowDateTime) / 10;
> #else
> -return av_gettime_relative();
> +time_stamps.user_usec = time_stamps.sys_usec = 0;
> #endif
> +return time_stamps;
> }
>
> static int64_t getmaxrss(void)
> @@ -4759,7 +4776,7 @@ static void log_callback_null(void *ptr, int level,
> const char *fmt, va_list vl)
> int main(int argc, char **argv)
> {
> int i, ret;
> -int64_t ti;
> +TimeStamps ti;
>
> init_dynload();
>
> @@ -4811,12 +4828,17 @@ int main(int argc, char **argv)
> want_sdp = 0;
> }
>
> -current_time = ti = getutime();
> +current_time = ti = get_time_stamps();
> if (transcode() < 0)
> exit_program(1);
> -ti = getutime() - ti;
> if (do_benchmark) {
> -av_log(NULL, AV_LOG_INFO, "bench: utime=%0.3fs\n", ti / 100.0);
> + current_time = get_time_stamps();
> + int64_t utime = current_time.user_usec - ti.user_usec;
> + int64_t stime = current_time.sys_usec - ti.sys_usec;
> + int64_t rtime = current_time.real_usec - ti.real_usec;
> + av_log(NULL, AV_LOG_INFO,
> + "bench: utime=%0.3fs stime=%0.3fs rtime=%0.3fs\n",
> + utime / 100.0, stime / 100.0, rtime / 100.0);
> }
> av_log(NULL, AV_LOG_DEBUG, "%"PRIu64" frames successfully decoded,
> %"PRIu64" decoding errors\n",
> decode_error_stat[0], decode_error_stat[1]);
Is there any reason you you can't just use /usr/bin/time?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] github
On Thu, 26 Apr 2018 08:59:03 -0800 Lou Logan wrote: > On Thu, Apr 26, 2018, at 4:40 AM, wm4 wrote: > > > > To be fair, I'd prefer the github issue tracker over TRAC any day. > > At least it isn't Bugzilla. What are some of the problems with trac? Is there > a self-hostable bug tracker you prefer? It's very slow, and the GUI sucks. It has no usable search functions, and trying to get a ticket list is a horror. Also its UI is cluttered bugzilla-style, although not as horribly as bugzilla. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/1] make headers compile in c++ mode
On Thu, 26 Apr 2018 14:51:09 +0200 Daniel Oberhoff wrote: > > On 26. Apr 2018, at 14:49, Nicolas George wrote: > > > > Daniel Oberhoff (2018-04-26): > >> --- > >> include/ffnvcodec/dynlink_loader.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > > I may be missing something obvious, but I am not seeing this file in our > > repository. > > sorry, its from the nv-codec-headers repo. Should have stated that in the > subject. Is this not the right list for patches there? The repository is a ffmpeg project, so this is the right place. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] github
On Thu, 26 Apr 2018 14:12:14 +0200 Hendrik Leppkes wrote: > On Thu, Apr 26, 2018 at 2:02 PM, Daniel Oberhoff > wrote: > > > >> Am 26.04.2018 um 13:59 schrieb Daniel Oberhoff > >> : > >> > >> > >>> Am 26.04.2018 um 13:56 schrieb Daniel Oberhoff > >>> : > >>> > >>> > Am 26.04.2018 um 13:52 schrieb Nicolas George : > > Daniel Oberhoff (2018-04-26): > > I was wondering if there is any chance to move development to github? > > I.e. not just mirror, but as primary development repo, with issues and > > pull requests? Would make collaboration a *lot* easier (think of > > submitting a pr instead of having to generate/format/split patches). > > If development involves working in a web browser a lot, count me out. > Can you point me to the command-line > >>> > >>> https://hub.github.com/hub.1.html > >> > >> But you can’t really do reviews that way, so the criticism stands. > > > > BTW, is there any kind of issue tracking? > > https://trac.ffmpeg.org/ To be fair, I'd prefer the github issue tracker over TRAC any day. Still has the other problems I mentioned. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] FFserver further development direction
On Thu, 26 Apr 2018 14:19:51 +0200 Nicolas George wrote: > Josh de Kock (2018-04-26): > > >>Generally, adding more things to public API is a bad move > > > What I mean by this is making private fields public is generally a bad move. > > They were initially made private for a reason, and if you suddenly need to > > modify them outside then you must ask: What does this new code do > > differently to all the other code which makes it require a private field? > > > > It's a matter of consistency, if some new code could be written without > > requiring a private field to become public then this is better. > > It's also a matter of complexity, the API is less complex if there are less > > public field. If it wasn't better then please submit a patch which makes all > > private fields public. Of course, this doesn't apply to everything sometimes > > there are genuine reasons for new API fields to be added but *generally* a > > smaller API leads to a better experience. I did say that in this case I was > > unsure. > > I think the statement you justified here is: > > "Generally, adding more things to public API *without need* is a bad > move." > > I agree with that statement. But I also agree with the statement: > > Generally, adding more thing to public API when needed is a good move. All public API was added because someone felt a need for it at one time in the past. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec: remove anatoliy prores encoder
On Thu, 26 Apr 2018 13:39:56 +0200 Carl Eugen Hoyos wrote: > 2018-04-26 13:34 GMT+02:00, Paul B Mahol : > > On 4/26/18, Carl Eugen Hoyos wrote: > >> 2018-04-26 13:17 GMT+02:00, Josh de Kock : > >>> On 2017/06/26 15:09, Paul B Mahol wrote: > Rationale: > - Slower then other encoder > - Less configurable > - Does not support alpha profile > - Does not set interlaced flag > - Worse output quality > - No need for 2 encoders > > Signed-off-by: Paul B Mahol > > >>> Is there any reason this was not pushed? > >>> I can't seem to see any argument against it. > >> > >> It was shown in the past that this encoder is faster, > >> more efficient and produces better quality. > > > > Why are you not telling real truth? > > This is surprisingly rude: > I am always trying to tell the truth, one of the things > that make me less happy about contributing here is > both that I am not allowed to write the truth anymore. What would those truths be? Note: insults are not truths. > Anyway: In the discussion about adding one of the > features you mention above, tests were posted that > showed this encoder to produce better (objective, > with all its disadvantages) quality using measurably > less cycles. > > > None of your claims are really true. > > Given how much you embarrassed us in the prores > discussion, I wonder why you make this claim;-) That is surprisingly rude. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] avcodec: remove anatoliy prores encoder
On Thu, 26 Apr 2018 13:34:12 +0200 Paul B Mahol wrote: > On 4/26/18, Carl Eugen Hoyos wrote: > > 2018-04-26 13:17 GMT+02:00, Josh de Kock : > >> On 2017/06/26 15:09, Paul B Mahol wrote: > >>> Rationale: > >>> - Slower then other encoder > >>> - Less configurable > >>> - Does not support alpha profile > >>> - Does not set interlaced flag > >>> - Worse output quality > >>> - No need for 2 encoders > >>> > >>> Signed-off-by: Paul B Mahol > >>> > >> Is there any reason this was not pushed? > >> I can't seem to see any argument against it. > > > > It was shown in the past that this encoder is faster, > > more efficient and produces better quality. > > Why are you not telling real truth? > > None of your claims are really true. > > I wonder why. Can any of you post numbers? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] github
On Thu, 26 Apr 2018 13:15:52 +0200 Daniel Oberhoff wrote: > Hello, > > I was wondering if there is any chance to move development to github? I.e. > not just mirror, but as primary development repo, with issues and pull > requests? Would make collaboration a *lot* easier (think of submitting a pr > instead of having to generate/format/split patches). Using git send-email is actually simpler than sending a github PR. Also github will delete or hide review comments once you force-push the PR branch (after you've fixed things that were pointed out). It would force everyone to use a complex and slow web interface. There's no easily searchable archive of past reviews, like the mailing list archive. There is also this aspect that github is an US company and a single point of failure. If there's a problem we couldn't just move the server somewhere else. Github would also be in control who's allowed to register and make posts, which seems rather strange to me. Another small aspect is that we've seen really awful low quality contributions on our github repository, even though we explicitly state that we don't do PRs. There have been 130 pull requests ever since we made a dummy PR that we ignore pull requests. Why open the flood gates? So I have doubts that it would work for us. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [GSoC] FFserver further development direction
On Thu, 26 Apr 2018 12:47:58 +0200 Nicolas George wrote: > Josh de Kock (2018-04-26): > >It's also not impossible, nor irrational to be removing code which > > doesn't fit or could be written better if an alternative is provided, the > > need was never there or removal can otherwise be justified. > > Then justify. But you will need to address all the arguments that were > given three years ago. > > The policy of this project has always been that for its core features it > must not depend on external libraries, apart from system libraries. What > exactly constitutes a "system library" is up for discussion, but in this > particular case there is no doubt. > > What constitutes a "core feature" is also up for debate, but ffserver > have been part of the project for much more time than not, have many > users that are upset by its removal, and was only removed because the > code was an obsolete mess and nobody had the courage to rework it. It > seems unambiguous to me that ffserver IS a core feature of the project. So the whole argument is: we need a HTTP server, because a "core feature" (which is really a separate program that uses that libav* libs) needs it? That's a pretty rigid rule. There are other external libraries we're relying on for relatively important functionality, and which can not really be called system libs: we support 3 or 4 TLS wrapper libs for HTTPS, libass for subtitle rendering, some libs for reading and rendering MIDI-like file formats, libx264 for h264 encoding (the most commonly used codec!), SDL for ffplay, and some other codec libs for which we have builtin implementations, because the builtin ones were too bad (AAC encoding used to be one). There's probably more. > I will add two considerations: > > First, the actual objections to having an HTTP server in the project are > about the maintenance burden, are they not? In that case, it seems to me > that the most weight in the decision should be given to people who have > an accurate idea of what it involves. So I ask: Do you consider yourself > skilled in network programming? Are you familiar with the RFCs > describing the HTTP protocol? Have you in the past implemented HTTP > servers? The HTTP _client_ in lavf is rather bad. I know because I get to use it with a lot of websites. And it doesn't even support HTTP 2. Just using libcurl or such would probably have saved us a lot of pain. API users can't just use their own HTTP client btw., because other lavf components depend on the native one too much. I have no doubt the server code would end up with even poorer support. The server code seems even barely used and was probably not battle-tested with a large number of different clients. Besides, it's a bad idea to extend the server APIs while the libavformat I/O API is in its current state that is much in need for cleanup, especially API-wise. Isn't the argument that we should not add premature API up your alley? In general badly NIH-ing something sufficiently complicated and that is not quite in the central focus of the project will end up worse than relying on completed external components that have spend a lot of time to complete it, including fixing any bugs and issues. > Second, the habit of endlessly going back on past decisions is really > hurting the project. I know I have several ambitious goals for FFmpeg, It was because the past decisions were not actually unanimous, so the contra-forces, who didn't accept or even recognize the past decisions, sometimes win back ground later. This happens because FFmpeg is a project full of in-fighting. > including a fully-typed option system with minimum string escaping. But > under the current conditions, there is no way I would ever start working > on them: we could decide today that the goal is worthy, but by the time > the first batch of development is done, some people will start > bieshedding it to death, and if it ever gets applied they will start > yapping "revert, revert" each time it causes a tiny bug. > > Speaking for myself, I can say this: I will not perform anything more > than basic maintenance on code under my responsibility as long as this > project has this "one step forward, two steps on the side, one step > backward" mentality; that would be a waste of my time. So if you think Well I don't think you're entirely innocent in that. > that my past work (especially the rework of the lavfi scheduling) had > merit, you should help finding a solution. That's some kind of passive aggressive threat. Please don't do this, because things like that only make the project more... difficult. > Electing a leader that can make final decisions and give the project > direction (no steps backward) would be one. > > I will finish by saying the same thing in a shorter way: > > Shut up, work and let people work. You never follow that though. You participate in endless flames, until the other side is tired and gives up. > Regards, > __
Re: [FFmpeg-devel] [GSoC] FFserver further development direction
On Tue, 24 Apr 2018 23:46:09 +0200 Nicolas George wrote: > Stephan Holljes (2018-04-24): > > The consensus seems to be that there are more disadvantages in using > > the http server of libavformat than there are advantages. > > I completely disagree. There is no point in having the HTTP server in > libavformat if it cannot be used to implement exactly that kind of > thing. Implementing ffserver with it is just the test bed that it > requires to become mature. > > The HTTP server in libavformat was accepted three years ago, and you > worked hard for it. Do not let people tell you it was for nothing. They > had their chance to discuss this three years ago. Sunk cost fallacy, and future maintenance overhead. > > This arose partly out of the discussion that there is no way to get a > > connected peer's address through the public API (as the filedescriptor > > is hidden in private structs). > > Well, then, let us add the functions that are needed in the public API. > It does not seem that difficult to design. > > Regards, > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/h264_slice: use the new SAR early when setting the decoder
On Wed, 25 Apr 2018 09:55:10 +0200
Steve Lhomme wrote:
> Le 24/04/2018 à 08:28, Hendrik Leppkes a écrit :
> > On Fri, Jan 19, 2018 at 1:01 PM, Steve Lhomme wrote:
> >> If we don't do that get_format might not be called for a while and the
> >> proper
> >> SAR not used.
> >>
> >> See the sample mentioned here: https://trac.videolan.org/vlc/ticket/19435
> >> ---
> >> libavcodec/h264_slice.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> >> index e6b7998834..319a37f5b6 100644
> >> --- a/libavcodec/h264_slice.c
> >> +++ b/libavcodec/h264_slice.c
> >> @@ -1050,8 +1050,10 @@ static int h264_init_ps(H264Context *h, const
> >> H264SliceContext *sl, int first_sl
> >> || (non_j_pixfmt(h->avctx->pix_fmt) !=
> >> non_j_pixfmt(get_pixel_format(h, 0
> >> must_reinit = 1;
> >>
> >> -if (first_slice && av_cmp_q(sps->sar, h->avctx->sample_aspect_ratio))
> >> +if (first_slice && av_cmp_q(sps->sar, h->avctx->sample_aspect_ratio))
> >> {
> >> must_reinit = 1;
> >> +ff_set_sar(h->avctx, sps->sar);
> >> +}
> >>
> >> if (!h->setup_finished) {
> >> h->avctx->profile = ff_h264_get_profile(sps);
> >> --
> >> 2.14.2
> > Why do we even do a get_format call for aspect ratio changes in the
> > first place? It doesn't change the physical image properties, the
> > format of image buffers etc remains the same, hwaccel decisions remain
> > the same.. anyone know why this is?
>
> How are we supposed to know the display format has changed ? Checking
> each AVFrame sample_aspect_ratio to see if something changed ?
Yes. get_format is for allocating memory only.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/4] avfilter/avfiltergraph: fix has_alpha in pick_format
On Tue, 24 Apr 2018 20:23:43 +0200 (CEST)
Marton Balint wrote:
> On Tue, 24 Apr 2018, Michael Niedermayer wrote:
>
> > On Tue, Apr 24, 2018 at 02:10:53AM +0200, Michael Niedermayer wrote:
> >> On Mon, Apr 23, 2018 at 04:50:54AM +0200, Marton Balint wrote:
> >>>
> >>>
> >>> On Mon, 23 Apr 2018, Michael Niedermayer wrote:
> >>>
> On Sun, Apr 22, 2018 at 01:44:19PM +0200, Marton Balint wrote:
> >
> >
> > On Fri, 20 Apr 2018, Michael Niedermayer wrote:
> >
> >> On Thu, Apr 19, 2018 at 09:32:19PM +0200, Marton Balint wrote:
> >>> A pixel format which has a palette also has alpha, without this patch
> >>> the
> >>> format negotiation might have preferred formats without alpha even if
> >>> the
> >>> source had alpha.
> >>>
> >>> Signed-off-by: Marton Balint
> >>> ---
> >>> libavfilter/avfiltergraph.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> >>> index 4cc6892404..e18f733e23 100644
> >>> --- a/libavfilter/avfiltergraph.c
> >>> +++ b/libavfilter/avfiltergraph.c
> >>> @@ -679,7 +679,7 @@ static int pick_format(AVFilterLink *link,
> >>> AVFilterLink *ref)
> >>>
> >>>if (link->type == AVMEDIA_TYPE_VIDEO) {
> >>>if(ref && ref->type == AVMEDIA_TYPE_VIDEO){
> >>> -int has_alpha=
> >>> av_pix_fmt_desc_get(ref->format)->nb_components % 2 == 0;
> >>> +int has_alpha=
> >>> av_pix_fmt_desc_get(ref->format)->nb_components % 2 == 0 ||
> >>> (av_pix_fmt_desc_get(ref->format)->flags & AV_PIX_FMT_FLAG_PAL);
> >>
> >> This causes various output files to grow in size with a unused alpha
> >> plane
> >>
> >> a random example: (there are likels better examples)
> >> ./ffmpeg -y -i ~/tickets/1116/1023.bmp -vf negate fileX.bmp
> >>
> >> Iam not sure unconditionally treating all palettes as if they have
> >> non fully opaque entries is a good idea.
> >
> > Obviously not, but it is already treated this way in most places.
> > Having a
> > bigger image with alpha is better than losing alpha. And the user can
> > always
> > force losing alpha with a filter, and sometimes he has to do that anyway
> > because for example fre0r filters have no way of signalling if they use
> > alpha or not...
>
> you can, the average user certainly doesnt have the knowledge to adjust
> anything alpha by hand, the average user isnt even aware of that the
> issue
> is alpha or pal8 related probably
>
> also about "better", i saw a few cases that got bigger, i dont remember
> seeing a case that was fixed.
> Have you seen real usecases this fixes ?
> >>>
> >>> A source file with a palette and alpha and a filter which supports formats
> >>> with both alpha and without:
> >>>
> >>> https://dab1nmslvvntp.cloudfront.net/images/blogs/design/8bit-trans.png
> >>>
> >>> ffmpeg -i 8bit-trans.png -vf negate out.bmp
> >>
> >> i assume fate doesnt cover this yet, so a new fate test probably makes
> >> sense
> >
> > i still think it would be more ideal if this is fully fixed
> > (by alpha / non alpha pal8) or other.
> >
> > Because as is we have a set of bugs, and with this patchset we fix some
> > while
> > introducing other (maybe less important though) bugs.
> > Then later behavior changes again when this is all fixed.
> > A smaller number of behavior changes should be better and less confusing to
> > users.
>
> I will rework the series, we can postpone actually fixing ffmpeg_filters.c
> and avfiltergraph.c pick_format(), I will add FIXME-s for those.
At this point, actually adding a separate PAL8 format seems most
attractive. Wouldn't it make everyone happy? Though I wonder which
codecs are supposed to use it. (And if it's not clear whether something
has alpha or not, we should strictly opt not to lose information,
anyway.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/h264_slice: use the new SAR early when setting the decoder
On Tue, 24 Apr 2018 08:28:35 +0200
Hendrik Leppkes wrote:
> On Fri, Jan 19, 2018 at 1:01 PM, Steve Lhomme wrote:
> > If we don't do that get_format might not be called for a while and the
> > proper
> > SAR not used.
> >
> > See the sample mentioned here: https://trac.videolan.org/vlc/ticket/19435
> > ---
> > libavcodec/h264_slice.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index e6b7998834..319a37f5b6 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1050,8 +1050,10 @@ static int h264_init_ps(H264Context *h, const
> > H264SliceContext *sl, int first_sl
> > || (non_j_pixfmt(h->avctx->pix_fmt) !=
> > non_j_pixfmt(get_pixel_format(h, 0
> > must_reinit = 1;
> >
> > -if (first_slice && av_cmp_q(sps->sar, h->avctx->sample_aspect_ratio))
> > +if (first_slice && av_cmp_q(sps->sar, h->avctx->sample_aspect_ratio)) {
> > must_reinit = 1;
> > +ff_set_sar(h->avctx, sps->sar);
> > +}
> >
> > if (!h->setup_finished) {
> > h->avctx->profile = ff_h264_get_profile(sps);
> > --
> > 2.14.2
>
> Why do we even do a get_format call for aspect ratio changes in the
> first place? It doesn't change the physical image properties, the
> format of image buffers etc remains the same, hwaccel decisions remain
> the same.. anyone know why this is?
Commit 5388f0b479c56179d566c49afd8765fefef4a18e.
https://ffmpeg.org/pipermail/ffmpeg-devel/2010-February/085931.html
I bet it's not a real reason anymore, but didn't try.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat: add mbedTLS based TLS
On Mon, 23 Apr 2018 21:09:09 +0200 Thomas Volkert wrote: > On 23.04.2018 14:08, Rostislav Pehlivanov wrote: > > On 23 April 2018 at 10:27, Thomas Volkert wrote: > > > >> On 22.04.2018 20:03, Carl Eugen Hoyos wrote: > >>> 2018-04-22 20:00 GMT+02:00, Nicolas George : > Carl Eugen Hoyos (2018-04-22): > > How do you detect that this is not the "current version" of mbed? > Is it really our responsibility? > >>> We try to always do it and I believe that allowing LGPL makes > >>> more sense and less headache: Since we do the checks so > >>> rigorously it makes sense to assume we did it as correctly > >>> for this case. > >>> > >>> I don't understand why we don't go the easy way that clearly > >>> has advantages instead for the complicated way (with at > >>> least some disadvantages). > >> Okay. I looked over their web page and the Debian packages again. > >> The web page of mbedTLS declares Apache license as the "primary open > >> source license". > >> > >> I will add it to EXTERNAL_LIBRARY_VERSION3_LIST and push it today, if > >> their are no further objections. > >> > >> Best regards, > >> Thomas. > >> ___ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >> > > I'd like some memory usage and performance comparisons to gnutls and > > The mbedTLS library uses less than half of the disc space compared to > OpenSSL in this context. > I don't have exact figures for memory consumption for different > scenarios right now. > > > > openssl before you push. This is the 6th (!!) TLS library we'd be > > supporting. > > Two of the existing ones are OS specific. > > In general, I see mbedTLS as an enrichment for FFmpeg. MbedTLS seems to > be a promising alternative to OpenSSL and other TLS implementations in > the context of embedded systems: the disk footprint is smaller out of > the box, the memory footprint is smaller, the library is intentionally > modular so that these can be reduced further at need > (https://tls.mbed.org/tiny-ssl-library, > https://tls.mbed.org/kb/how-to/reduce-mbedtls-memory-and-storage-footprint). > > Additionally, in the planned context the mbedTLS library is already used > for signaling out-of-band control messages between network peers. And it > is planned to do some more tuning to the overall setup to get an even > smaller version of mbedTLS on the target device. To be honest that's not really convincing. How does it compare with e.g. libtls? In my opinion it's probably not wrong to give another TLS lib a try, but: 1. It was kind of too early. You should at least have given us some time to respond. 2. Having many TLS wrappers is actually not a good thing because it will confuse users about which one to pick, and the wrappers will all have subtly different behavior that can become a problem for ffmpeg and libav* users. 3. It's hard to remove redundant things even after it has turned out that they were not that useful. (See e.g. how much effort it was to remove some of the virtually useless codec lib wrappers.) But it's just my opinion that having fewer well maintained choices is better than many with maintenance deficits. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 3/4] avutil/pixdesc: add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8
On Mon, 23 Apr 2018 20:25:40 +0200 (CEST) Marton Balint wrote: > On Sun, 22 Apr 2018, wm4 wrote: > > > On Sun, 22 Apr 2018 13:24:11 +0200 (CEST) > > Marton Balint wrote: > > > >> On Fri, 20 Apr 2018, wm4 wrote: > >> > >> > On Thu, 19 Apr 2018 23:25:03 +0200 > >> > Marton Balint wrote: > >> > > >> >> Signed-off-by: Marton Balint > >> >> --- > >> >> doc/APIchanges| 3 +++ > >> >> libavutil/pixdesc.c | 3 +-- > >> >> libavutil/pixdesc.h | 8 ++-- > >> >> libavutil/tests/pixdesc.c | 4 > >> >> libavutil/version.h | 2 +- > >> >> 5 files changed, 7 insertions(+), 13 deletions(-) > >> >> > >> >> diff --git a/doc/APIchanges b/doc/APIchanges > >> >> index 4f6ac2a031..d9b457e080 100644 > >> >> --- a/doc/APIchanges > >> >> +++ b/doc/APIchanges > >> >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >> >> > >> >> API changes, most recent first: > >> >> > >> >> +2018-04-xx - xx - lavu 56.16.100 - pixdesc.h > >> >> + Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8. > >> >> + > >> > >> [..] > >> > >> > > >> > Probably fine. While I like it, we also have to be careful about the > >> > consequences. Does it change FATE or the results of that pixfmt choosing > >> > function, avcodec_find_best_pix_fmt_of_list()? > >> > >> Fate passes. I am not sure about avcodec_find_best_pix_fmt_of_list(), but > >> since pixdesc_has_alpha() in avutil/pixdesc.c already considered PAL8 as a > >> format with alpha, I don't think it changes. > > > > Oh, interesting point. So this whole discussion is moot anyway, since > > it always suggested RGBA when converting PAL8 to a non-paletted RGB > > format? > > Only if has_alpha was true when the user called > avcodec_find_best_pix_fmt_of_list() or av_find_best_pix_fmt_of_2(). > > For avfiltergraph format negotiation or for ffmpeg.c automatic codec pixel > format selection, pal8 was not considered as a format with alpha > (nb_components % 2 == 0 check was used), so they called > av_find_best_pix_fmt_of_2() with has_alpha=false, therefore alpha loss was > not penalized. A subsequent patch takes care of that. Right. That's a bit of a mess. So if anyone thinks that's a problem, the avfilter code can still be special-cased, or we could introduce a non-alpha PAL format. LGTM anyway, it's a nice cleanup. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] lavu/opt: add AV_OPT_FLAG_DEPRECATED
On Sun, 22 Apr 2018 16:38:08 +0200
Clément Bœsch wrote:
> ---
> TODO: APIChanges + lavu minor bump (not done yet because it will
> conflict with the threadmessage patch)
> ---
> libavutil/opt.c | 6 ++
> libavutil/opt.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/libavutil/opt.c b/libavutil/opt.c
> index 3b0aab4ee8..99282605f5 100644
> --- a/libavutil/opt.c
> +++ b/libavutil/opt.c
> @@ -463,6 +463,9 @@ int av_opt_set(void *obj, const char *name, const char
> *val, int search_flags)
> if (o->flags & AV_OPT_FLAG_READONLY)
> return AVERROR(EINVAL);
>
> +if (o->flags & AV_OPT_FLAG_DEPRECATED)
> +av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n",
> name, o->help);
> +
> dst = ((uint8_t *)target_obj) + o->offset;
> switch (o->type) {
> case AV_OPT_TYPE_BOOL:
> @@ -759,6 +762,9 @@ int av_opt_get(void *obj, const char *name, int
> search_flags, uint8_t **out_val)
> if (!o || !target_obj || (o->offset<=0 && o->type != AV_OPT_TYPE_CONST))
> return AVERROR_OPTION_NOT_FOUND;
>
> +if (o->flags & AV_OPT_FLAG_DEPRECATED)
> +av_log(obj, AV_LOG_WARNING, "The \"%s\" option is deprecated: %s\n",
> name, o->help);
> +
> dst = (uint8_t *)target_obj + o->offset;
>
> buf[0] = 0;
> diff --git a/libavutil/opt.h b/libavutil/opt.h
> index 07da68ea23..1352741fb6 100644
> --- a/libavutil/opt.h
> +++ b/libavutil/opt.h
> @@ -289,6 +289,7 @@ typedef struct AVOption {
> #define AV_OPT_FLAG_READONLY128
> #define AV_OPT_FLAG_BSF_PARAM (1<<8) ///< a generic parameter which
> can be set by the user for bit stream filtering
> #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which
> can be set by the user for filtering
> +#define AV_OPT_FLAG_DEPRECATED (1<<17)
> //FIXME think about enc-audio, ... style flags
>
> /**
Seems like a good idea.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 3/4] avutil/pixdesc: add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8
On Sun, 22 Apr 2018 13:24:11 +0200 (CEST) Marton Balint wrote: > On Fri, 20 Apr 2018, wm4 wrote: > > > On Thu, 19 Apr 2018 23:25:03 +0200 > > Marton Balint wrote: > > > >> Signed-off-by: Marton Balint > >> --- > >> doc/APIchanges| 3 +++ > >> libavutil/pixdesc.c | 3 +-- > >> libavutil/pixdesc.h | 8 ++-- > >> libavutil/tests/pixdesc.c | 4 > >> libavutil/version.h | 2 +- > >> 5 files changed, 7 insertions(+), 13 deletions(-) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index 4f6ac2a031..d9b457e080 100644 > >> --- a/doc/APIchanges > >> +++ b/doc/APIchanges > >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >> > >> API changes, most recent first: > >> > >> +2018-04-xx - xx - lavu 56.16.100 - pixdesc.h > >> + Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8. > >> + > > [..] > > > > > Probably fine. While I like it, we also have to be careful about the > > consequences. Does it change FATE or the results of that pixfmt choosing > > function, avcodec_find_best_pix_fmt_of_list()? > > Fate passes. I am not sure about avcodec_find_best_pix_fmt_of_list(), but > since pixdesc_has_alpha() in avutil/pixdesc.c already considered PAL8 as a > format with alpha, I don't think it changes. Oh, interesting point. So this whole discussion is moot anyway, since it always suggested RGBA when converting PAL8 to a non-paletted RGB format? > > Are there any formats > > that decode to PAL8, but write garbage to the alpha component of the > > palette? > > If there are, then those should be fixed. The damage was already done when > it was decided to consider PAL8 a format with alpha, so in some cases > garbage was already used... Agreed. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/4] avfilter/avfiltergraph: fix has_alpha in pick_format
On Sun, 22 Apr 2018 13:44:19 +0200 (CEST)
Marton Balint wrote:
> On Fri, 20 Apr 2018, Michael Niedermayer wrote:
>
> > On Thu, Apr 19, 2018 at 09:32:19PM +0200, Marton Balint wrote:
> >> A pixel format which has a palette also has alpha, without this patch the
> >> format negotiation might have preferred formats without alpha even if the
> >> source had alpha.
> >>
> >> Signed-off-by: Marton Balint
> >> ---
> >> libavfilter/avfiltergraph.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavfilter/avfiltergraph.c b/libavfilter/avfiltergraph.c
> >> index 4cc6892404..e18f733e23 100644
> >> --- a/libavfilter/avfiltergraph.c
> >> +++ b/libavfilter/avfiltergraph.c
> >> @@ -679,7 +679,7 @@ static int pick_format(AVFilterLink *link,
> >> AVFilterLink *ref)
> >>
> >> if (link->type == AVMEDIA_TYPE_VIDEO) {
> >> if(ref && ref->type == AVMEDIA_TYPE_VIDEO){
> >> -int has_alpha=
> >> av_pix_fmt_desc_get(ref->format)->nb_components % 2 == 0;
> >> +int has_alpha=
> >> av_pix_fmt_desc_get(ref->format)->nb_components % 2 == 0 ||
> >> (av_pix_fmt_desc_get(ref->format)->flags & AV_PIX_FMT_FLAG_PAL);
> >
> > This causes various output files to grow in size with a unused alpha plane
> >
> > a random example: (there are likels better examples)
> > ./ffmpeg -y -i ~/tickets/1116/1023.bmp -vf negate fileX.bmp
> >
> > Iam not sure unconditionally treating all palettes as if they have
> > non fully opaque entries is a good idea.
>
> Obviously not, but it is already treated this way in most places. Having a
> bigger image with alpha is better than losing alpha. And the user can
> always force losing alpha with a filter, and sometimes he has to do that
> anyway because for example fre0r filters have no way of signalling if they
> use alpha or not...
>
> >
> > Doesnt this patchset replace a problem by another problem ?
> > It may be better to not rush this and find a complete solution
> >
> > a 2nd pixfmt and or scaning the 256 entries for their alpha values
> > is maybe the direction we could go
>
> I don't think scaning can work for anything other than still images, as
> the palette can change per-frame AFAIK. Introducing a new pixel format
> seems the better approach, however keeping in mind compatibility and
> existing code, I'd rather make the newly introduced one the one without
> alpha, and move things gradually to it. Still seems like a fair amount of
> work...
PAL8 has the same problem as RGBA formats: you can't know whether it's
going to use alpha or not. (Indeed, there are codecs which can update
the palette, and who says filters won't change the palette?) With PAL8
it's just faster to verify for a particular frame. But in general, you
have to know in advance, and have to signal it in advance. I think that
works relatively well with PIXFMT_RGBA vs. PIXFMT_RGB0.
So I would argue it's fine to change PAL8 to having alpha, and if we
find cases that would really benefit from it, add PIXFMT_PAL8_RGB0. (Or
_RGBX or _NO_ALPHA or whatever.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/dashdec: Do not use memcpy() to copy a struct
On Sun, 22 Apr 2018 08:41:18 +0800
Liu Steven wrote:
> > 在 2018年4月22日，上午5:23，wm4 写道：
> >
> > On Sat, 21 Apr 2018 22:55:33 +0200
> > Carl Eugen Hoyos wrote:
> >
> >> 2018-04-19 4:45 GMT+02:00, Steven Liu :
> >>>
> >>>
> >>>> On 19 Apr 2018, at 03:20, wm4 wrote:
> >>>>
> >>>> On Wed, 18 Apr 2018 16:10:26 -0300
> >>>> James Almer wrote:
> >>>>
> >>>>> On 4/18/2018 2:45 PM, Carl Eugen Hoyos wrote:
> >>>>>> Hi!
> >>>>>>
> >>>>>> Attached patch is supposed to fix a warning (and a bug), is this the
> >>>>>> right and preferred fix?
> >>>>>>
> >>>>>> Please comment, Carl Eugen
> >>>>>>
> >>>>>>
> >>>>>> 0001-lavf-dashdec-Do-not-use-memcpy-to-copy-a-struct.patch
> >>>>>>
> >>>>>>
> >>>>>> From cf7d2aefc1a3b3a2e9f578ede43906ed6ee96bfd Mon Sep 17 00:00:00 2001
> >>>>>> From: Carl Eugen Hoyos
> >>>>>> Date: Wed, 18 Apr 2018 19:42:57 +0200
> >>>>>> Subject: [PATCH] lavf/dashdec: Do not use memcpy() to copy a struct.
> >>>>>>
> >>>>>> Fixes a warning:
> >>>>>> libavformat/dashdec.c:1900:65: warning: argument to 'sizeof' in
> >>>>>> 'memcpy'
> >>>>>> call is the same pointer type 'struct fragment *' as the destination;
> >>>>>> expected 'struct fragment' or an explicit length
> >>>>>> ---
> >>>>>> libavformat/dashdec.c |2 +-
> >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> >>>>>> index 6304ad9..917fb54 100644
> >>>>>> --- a/libavformat/dashdec.c
> >>>>>> +++ b/libavformat/dashdec.c
> >>>>>> @@ -1897,7 +1897,7 @@ static int init_section_compare_audio(DASHContext
> >>>>>> *c)
> >>>>>>
> >>>>>> static void copy_init_section(struct representation *rep_dest, struct
> >>>>>> representation *rep_src)
> >>>>>> {
> >>>>>> -memcpy(rep_dest->init_section, rep_src->init_section,
> >>>>>> sizeof(rep_src->init_section));
> >>>>>> +rep_dest->init_section = rep_src->init_section;
> >>>>>
> >>>>> This would only copy the pointer. The fact memcpy was used here makes me
> >>>>> think the intention was to copy the contents of the struct, so something
> >>>>> like
> >>>>>
> >>>>> *rep_dest->init_section = *rep_src->init_section;
> >>>>>
> >>>>> or
> >>>>>
> >>>>> memcpy(rep_dest->init_section, rep_src->init_section,
> >>>>> sizeof(*rep_src->init_section));
> >>>>>
> >>>>> Would be the correct fix.
> >>>>
> >>>> The first version would be preferable. But I think the original code
> >>>> makes no sense and was never really tested. Looking slightly closer at
> >>>> the code, init_section points to a struct that contains a further
> >>>> pointer, which would require allocating and dup'ing the memory.
> >>>>
> >>>> Also the rep_dest->init_sec_buf allocation call isn't even checked. It
> >>>> just memcpy's to a NULL pointer. This is some seriously shit code, and
> >>>> all of dashdec.c is shit. I'd like to ask Steven Liu (who
> >>>> reviewed/pushed the patch that added this copy_init_section code) to
> >>>> _actually_ review the patches and to keep up the quality standards in
> >>>> this project (which are slightly higher than this).
> >>> Yes, that is my mistake, patch welcome and welcome you to contribute code
> >>> for refine the dashdec
> >
> > The problem was that you didn't actually review the patch. There's
> > really no excuse for the code that has been added. It's not even valid
> > C. It's sort of your responsibility to make sure this doesn't happen.
> > Sorry if my words were a bit too direct, but for very new code dashdec
> > has a bit too many issues than it should have. Reviewing means more
> > than just replying "LGTM" and pushing a patch.
> The problem is how do you check i have not check the patch is ok or not?
> Only myself review the patch, where are the other guys when i response LGTM?
> you guys can objections the patch, but no, isn’t it?
You read it carefully, maybe apply the thing locally, and build and
test it. Reviewing basically means checking a patch for errors. Point
out the errors by replying to the patch, and ask the patch author to
fix them.
The more you can trust the patch author the more checks you can skip,
but generally you should at least skim the patch for things that look
wrong. In this case there were a bunch of obviously suspicious things,
including things we don't normally do (like the missing memory
allocation failure check). Looking whether a patch causes new compiler
warnings is also a thing worth to do.
And yes, this can be significant work, but it's all so to make sure bugs
are avoided, and that nobody has to fix the mess later.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/dashdec: Do not use memcpy() to copy a struct
On Sat, 21 Apr 2018 17:56:27 -0400 "Helmut K. C. Tessarek" wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA512 > > > On 2018-04-21 17:23, wm4 wrote: > > Just by the way, some have lamented that they think of it as > > "doxing" when you post my real name on this list. > > I don't really care, if somebody is using a pseudonym or their real name. Well I'm just saying how others interpreted CE's behavior. > However, I have to say that if sombody is using not their real name, > especially when their tone is often agressive and unfiltered, it > rather begs the question why that is. > > Is that person afraid that an employer might come across some of this > crap when doing research on a potential candidate? > Is that person not confident enough to stand by their opinion. > > Seriously, what is it? > > Btw, I myself often use blunt comments, which are mostly seen and read > as harsh, undiplomatic, and rude. I hate poltical correctness and love > sarcasm. Non of these traits are a plus in public. People love sugar > coating and talking around a problem, all in the name of diplomacy. It would be a big bother to change everything. Often you need a pseudonym anyway, because account names are handier when they are short. So why throw 2 names around? Also this email address was meant as spam sink in the first place so it has no meaningful name (registering to dozens of sites and mailing lists which all show the mail address in plain text, which meant it'd likely get a lot of spam). And one thing I don't want to do for sure is giving Google my name for free. So I'd probably want to change my email and email service too if I were to use my real name on this list. Regarding employers, they'd get my "pseudonym" anyway because I'd put a link to my github repository on my resume. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/dashdec: Do not use memcpy() to copy a struct
On Sat, 21 Apr 2018 23:30:35 +0200 Carl Eugen Hoyos wrote: > 2018-04-21 23:23 GMT+02:00, wm4 : > > >> No (independently of what I think of Vincent's character and tone). > > > > Agreed, independently of what I think of you. > > > > Just by the way, some have lamented that they think of it as "doxing" > > What is "doxing"? Something which most codes of conduct forbid. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/dashdec: Do not use memcpy() to copy a struct
On Sat, 21 Apr 2018 22:55:33 +0200
Carl Eugen Hoyos wrote:
> 2018-04-19 4:45 GMT+02:00, Steven Liu :
> >
> >
> >> On 19 Apr 2018, at 03:20, wm4 wrote:
> >>
> >> On Wed, 18 Apr 2018 16:10:26 -0300
> >> James Almer wrote:
> >>
> >>> On 4/18/2018 2:45 PM, Carl Eugen Hoyos wrote:
> >>>> Hi!
> >>>>
> >>>> Attached patch is supposed to fix a warning (and a bug), is this the
> >>>> right and preferred fix?
> >>>>
> >>>> Please comment, Carl Eugen
> >>>>
> >>>>
> >>>> 0001-lavf-dashdec-Do-not-use-memcpy-to-copy-a-struct.patch
> >>>>
> >>>>
> >>>> From cf7d2aefc1a3b3a2e9f578ede43906ed6ee96bfd Mon Sep 17 00:00:00 2001
> >>>> From: Carl Eugen Hoyos
> >>>> Date: Wed, 18 Apr 2018 19:42:57 +0200
> >>>> Subject: [PATCH] lavf/dashdec: Do not use memcpy() to copy a struct.
> >>>>
> >>>> Fixes a warning:
> >>>> libavformat/dashdec.c:1900:65: warning: argument to 'sizeof' in 'memcpy'
> >>>> call is the same pointer type 'struct fragment *' as the destination;
> >>>> expected 'struct fragment' or an explicit length
> >>>> ---
> >>>> libavformat/dashdec.c |2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> >>>> index 6304ad9..917fb54 100644
> >>>> --- a/libavformat/dashdec.c
> >>>> +++ b/libavformat/dashdec.c
> >>>> @@ -1897,7 +1897,7 @@ static int init_section_compare_audio(DASHContext
> >>>> *c)
> >>>>
> >>>> static void copy_init_section(struct representation *rep_dest, struct
> >>>> representation *rep_src)
> >>>> {
> >>>> -memcpy(rep_dest->init_section, rep_src->init_section,
> >>>> sizeof(rep_src->init_section));
> >>>> +rep_dest->init_section = rep_src->init_section;
> >>>
> >>> This would only copy the pointer. The fact memcpy was used here makes me
> >>> think the intention was to copy the contents of the struct, so something
> >>> like
> >>>
> >>> *rep_dest->init_section = *rep_src->init_section;
> >>>
> >>> or
> >>>
> >>> memcpy(rep_dest->init_section, rep_src->init_section,
> >>> sizeof(*rep_src->init_section));
> >>>
> >>> Would be the correct fix.
> >>
> >> The first version would be preferable. But I think the original code
> >> makes no sense and was never really tested. Looking slightly closer at
> >> the code, init_section points to a struct that contains a further
> >> pointer, which would require allocating and dup'ing the memory.
> >>
> >> Also the rep_dest->init_sec_buf allocation call isn't even checked. It
> >> just memcpy's to a NULL pointer. This is some seriously shit code, and
> >> all of dashdec.c is shit. I'd like to ask Steven Liu (who
> >> reviewed/pushed the patch that added this copy_init_section code) to
> >> _actually_ review the patches and to keep up the quality standards in
> >> this project (which are slightly higher than this).
> > Yes, that is my mistake, patch welcome and welcome you to contribute code
> > for refine the dashdec
The problem was that you didn't actually review the patch. There's
really no excuse for the code that has been added. It's not even valid
C. It's sort of your responsibility to make sure this doesn't happen.
Sorry if my words were a bit too direct, but for very new code dashdec
has a bit too many issues than it should have. Reviewing means more
than just replying "LGTM" and pushing a patch.
> No (independently of what I think of Vincent's character and tone).
Agreed, independently of what I think of you.
Just by the way, some have lamented that they think of it as "doxing"
when you post my real name on this list. Do you think this is acceptable
behavior? Don't worry, my real name has been on my github profile for
years (and before that on the mplayer2 website), in both cases visible
to anyone, so you don't have to fear that your childish attempts to
achieve whatever have any effect.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] doc/examples/hw_decode: Remove setting deprecated refcounted_frames
On Sat, 21 Apr 2018 15:54:40 +0800 Jun Zhao wrote: > When use new decode APIs(avcodec_send_packet/avcodec_receive_frame), > don't need to setting the deprecated field refcounted_frames. > > Signed-off-by: Jun Zhao > --- > doc/examples/hw_decode.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c > index 77ae8df..4a4e7fc 100644 > --- a/doc/examples/hw_decode.c > +++ b/doc/examples/hw_decode.c > @@ -211,7 +211,6 @@ int main(int argc, char *argv[]) > return -1; > > decoder_ctx->get_format = get_hw_format; > -av_opt_set_int(decoder_ctx, "refcounted_frames", 1, 0); > > if (hw_decoder_init(decoder_ctx, type) < 0) > return -1; 1-3 LGTM ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/videotoolbox: fix kVTCouldNotFindVideoDecoderErr trying to decode HEVC on iOS
On Fri, 20 Apr 2018 03:43:30 -0500 Rodger Combs wrote: > If there was a way to indicate this to consumers, or expose an option to turn > this off, I'd say it should have that... but there's no good infrastructure > to do that in an hwaccel, so whatever. > One nit; otherwise LGTM. > I guess it's OK as long as the Apple decoder really is faster than FFmpeg with frame threading, and it supports all features a hw decoder also would. The whole point of hwaccel is to get a speed advantage over the internal decoder. Some mechanism to indicate that a sw decoder is used (even if it's "better") would still be good. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCHv2 3/4] avutil/pixdesc: add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8
On Thu, 19 Apr 2018 23:25:03 +0200
Marton Balint wrote:
> Signed-off-by: Marton Balint
> ---
> doc/APIchanges| 3 +++
> libavutil/pixdesc.c | 3 +--
> libavutil/pixdesc.h | 8 ++--
> libavutil/tests/pixdesc.c | 4
> libavutil/version.h | 2 +-
> 5 files changed, 7 insertions(+), 13 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4f6ac2a031..d9b457e080 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> +2018-04-xx - xx - lavu 56.16.100 - pixdesc.h
> + Add AV_PIX_FMT_FLAG_ALPHA to AV_PIX_FMT_PAL8.
> +
> 8< - FFmpeg 4.0 was cut here 8< -
>
> 2018-04-03 - d6fc031caf - lavu 56.13.100 - pixdesc.h
> diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c
> index 8ed52751c1..a8be7b66e6 100644
> --- a/libavutil/pixdesc.c
> +++ b/libavutil/pixdesc.c
> @@ -288,7 +288,7 @@ static const AVPixFmtDescriptor
> av_pix_fmt_descriptors[AV_PIX_FMT_NB] = {
> .comp = {
> { 0, 1, 0, 0, 8, 0, 7, 1 },
> },
> -.flags = AV_PIX_FMT_FLAG_PAL,
> +.flags = AV_PIX_FMT_FLAG_PAL | AV_PIX_FMT_FLAG_ALPHA,
> },
> [AV_PIX_FMT_YUVJ420P] = {
> .name = "yuvj420p",
> @@ -2432,7 +2432,6 @@ void ff_check_pixfmt_descriptors(void){
> av_assert0(d->log2_chroma_h <= 3);
> av_assert0(d->nb_components <= 4);
> av_assert0(d->name && d->name[0]);
> -av_assert0((d->nb_components==4 || d->nb_components==2) ==
> !!(d->flags & AV_PIX_FMT_FLAG_ALPHA));
> av_assert2(av_get_pix_fmt(d->name) == i);
>
> for (j=0; jcomp); j++) {
> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
> index 1ab372782a..4f9c5a271f 100644
> --- a/libavutil/pixdesc.h
> +++ b/libavutil/pixdesc.h
> @@ -167,12 +167,8 @@ typedef struct AVPixFmtDescriptor {
>
> /**
> * The pixel format has an alpha channel. This is set on all formats that
> - * support alpha in some way. The exception is AV_PIX_FMT_PAL8, which can
> - * carry alpha as part of the palette. Details are explained in the
> - * AVPixelFormat enum, and are also encoded in the corresponding
> - * AVPixFmtDescriptor.
> - *
> - * The alpha is always straight, never pre-multiplied.
> + * support alpha in some way, including AV_PIX_FMT_PAL8. The alpha is always
> + * straight, never pre-multiplied.
> *
> * If a codec or a filter does not support alpha, it should set all alpha to
> * opaque, or use the equivalent pixel formats without alpha component, e.g.
> diff --git a/libavutil/tests/pixdesc.c b/libavutil/tests/pixdesc.c
> index 7fbfeea96c..34e2bea932 100644
> --- a/libavutil/tests/pixdesc.c
> +++ b/libavutil/tests/pixdesc.c
> @@ -37,10 +37,6 @@ int main(void){
> skip = 0;
> }
> av_log(NULL, AV_LOG_INFO, "pix fmt %s avg_bpp:%d colortype:%d\n",
> desc->name, av_get_padded_bits_per_pixel(desc), get_color_type(desc));
> -if ((!(desc->flags & AV_PIX_FMT_FLAG_ALPHA)) != (desc->nb_components
> != 2 && desc->nb_components != 4)) {
> -av_log(NULL, AV_LOG_ERROR, "Alpha flag mismatch\n");
> -err = 1;
> -}
> }
> return err;
> }
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 387421775f..23567000a3 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 56
> -#define LIBAVUTIL_VERSION_MINOR 15
> +#define LIBAVUTIL_VERSION_MINOR 16
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
Probably fine. While I like it, we also have to be careful about the
consequences. Does it change FATE or the results of that pixfmt choosing
function, avcodec_find_best_pix_fmt_of_list()? Are there any formats
that decode to PAL8, but write garbage to the alpha component of the
palette?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] libavformat/http: Refactor and fix additional leaks in get_cookies.
On Thu, 19 Apr 2018 12:55:00 -0700
rshaf...@tunein.com wrote:
> From: Richard Shaffer
>
> This refactors get_cookies to simplify some code paths, specifically for
> skipping logic in the while loop or exiting it. It also simplifies the logic
> for appending additional values to *cookies by replacing
> strlen/malloc/snprintf
> with one call av_asnprintf.
>
> This refactor fixes a bug where the cookie_params AVDictionary would get
> leaked
> if we failed to allocate a new buffer for writing to *cookies.
> ---
> Updated so that next = set_cookies = av_strdup(s->cookies) assignment is done
> on a separate line instead of inside the if conditional.
>
> libavformat/http.c | 65
> +++---
> 1 file changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index b4a1919f24..d59ffbbbe8 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1015,7 +1015,8 @@ static int process_line(URLContext *h, char *line, int
> line_count,
> /**
> * Create a string containing cookie values for use as a HTTP cookie header
> * field value for a particular path and domain from the cookie values
> stored in
> - * the HTTP protocol context. The cookie string is stored in *cookies.
> + * the HTTP protocol context. The cookie string is stored in *cookies, and
> may
> + * be NULL if there are no valid cookies.
> *
> * @return a negative value if an error condition occurred, 0 otherwise
> */
> @@ -1025,15 +1026,20 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> // cookie strings will look like Set-Cookie header field values.
> Multiple
> // Set-Cookie fields will result in multiple values delimited by a
> newline
> int ret = 0;
> -char *cookie, *set_cookies = av_strdup(s->cookies), *next = set_cookies;
> -
> -if (!set_cookies) return AVERROR(EINVAL);
> +char *cookie, *set_cookies, *next;
>
> // destroy any cookies in the dictionary.
> av_dict_free(&s->cookie_dict);
>
> +if (!s->cookies)
> +return 0;
> +
> +next = set_cookies = av_strdup(s->cookies);
> +if (!next)
> +return AVERROR(ENOMEM);
> +
> *cookies = NULL;
> -while ((cookie = av_strtok(next, "\n", &next))) {
> +while ((cookie = av_strtok(next, "\n", &next)) && !ret) {
> AVDictionary *cookie_params = NULL;
> AVDictionaryEntry *cookie_entry, *e;
>
> @@ -1043,23 +1049,19 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
>
> // continue on to the next cookie if this one cannot be parsed
> if (parse_set_cookie(cookie, &cookie_params))
> -continue;
> +goto skip_cookie;
>
> // if the cookie has no value, skip it
> cookie_entry = av_dict_get(cookie_params, "", NULL,
> AV_DICT_IGNORE_SUFFIX);
> -if (!cookie_entry || !cookie_entry->value) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (!cookie_entry || !cookie_entry->value)
> +goto skip_cookie;
>
> // if the cookie has expired, don't add it
> if ((e = av_dict_get(cookie_params, "expires", NULL, 0)) &&
> e->value) {
> struct tm tm_buf = {0};
> if (!parse_set_cookie_expiry_time(e->value, &tm_buf)) {
> -if (av_timegm(&tm_buf) < av_gettime() / 100) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (av_timegm(&tm_buf) < av_gettime() / 100)
> +goto skip_cookie;
> }
> }
>
> @@ -1067,42 +1069,31 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> if ((e = av_dict_get(cookie_params, "domain", NULL, 0)) && e->value)
> {
> // find the offset comparison is on the min domain (b.com, not
> a.b.com)
> int domain_offset = strlen(domain) - strlen(e->value);
> -if (domain_offset < 0) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (domain_offset < 0)
> +goto skip_cookie;
>
> // match the cookie domain
> -if (av_strcasecmp(&domain[domain_offset], e->value)) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (av_strcasecmp(&domain[domain_offset], e->value))
> +goto skip_cookie;
> }
>
> // ensure this cookie matches the path
> e = av_dict_get(cookie_params, "path", NULL, 0);
> -if (!e || av_strncasecmp(path, e->value, strlen(e->value))) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (!e || av_strncasecmp(path, e->value, strlen(e->value)))
> +goto skip_cookie;
>
> // cookie par
Re: [FFmpeg-devel] [PATCH 3/4] avutil/pixdesc: add av_pix_fmt_desc_has_alpha()
On Thu, 19 Apr 2018 21:44:36 +0200
Hendrik Leppkes wrote:
> ex
>
> On Thu, Apr 19, 2018 at 9:39 PM, wm4 wrote:
> > On Thu, 19 Apr 2018 21:32:20 +0200
> > Marton Balint wrote:
> >
> >> Signed-off-by: Marton Balint
> >> ---
> >> doc/APIchanges | 3 +++
> >> libavutil/pixdesc.h | 11 +++
> >> libavutil/version.h | 2 +-
> >> 3 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/doc/APIchanges b/doc/APIchanges
> >> index 4f6ac2a031..2a0b6f057a 100644
> >> --- a/doc/APIchanges
> >> +++ b/doc/APIchanges
> >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> >>
> >> API changes, most recent first:
> >>
> >> +2018-04-xx - xx - lavu 56.16.100 - pixdesc.h
> >> + Add av_pix_fmt_desc_has_alpha().
> >> +
> >> 8< - FFmpeg 4.0 was cut here 8< -
> >>
> >> 2018-04-03 - d6fc031caf - lavu 56.13.100 - pixdesc.h
> >> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
> >> index 1ab372782a..aef4313ccb 100644
> >> --- a/libavutil/pixdesc.h
> >> +++ b/libavutil/pixdesc.h
> >> @@ -430,4 +430,15 @@ int av_get_pix_fmt_loss(enum AVPixelFormat
> >> dst_pix_fmt,
> >> enum AVPixelFormat av_find_best_pix_fmt_of_2(enum AVPixelFormat
> >> dst_pix_fmt1, enum AVPixelFormat dst_pix_fmt2,
> >> enum AVPixelFormat
> >> src_pix_fmt, int has_alpha, int *loss_ptr);
> >>
> >> +/**
> >> + * Return true if a pixel format descriptor has alpha channel.
> >> + *
> >> + * @param desc the pixel format descriptor
> >> + * @return 1 if the pixel format descriptor has alpha, 0 otherwise.
> >> + */
> >> +static inline int av_pix_fmt_desc_has_alpha(const AVPixFmtDescriptor
> >> *desc)
> >> +{
> >> +return desc->nb_components == 2 || desc->nb_components == 4 ||
> >> (desc->flags & AV_PIX_FMT_FLAG_PAL);
> >> +}
> >
> > Good, idea, but...
> >
> > That doesn't seem very correct. Use AV_PIX_FMT_FLAG_ALPHA? (Although
> > PAL8 doesn't have it set, which is probably a bug. Or should have have
> > a separate PALA8 format?)
> >
>
> I agree, we should be using the flag we already have - and at that
> point, we probably also don't need a function to check it?
Yeah, except the weird PAL8 case.
> (The comment above AV_PIX_FMT_FLAG_ALPHA tries to explain the PAL8
> case - basically it is unknown if the palette contains alpha or not?
> or whatever)
It makes no sense to me. We had ambiguous formats before, like
AV_PIX_FMT_RGBA. The last component could be either alpha or padding.
Then AV_PIX_FMT_RGB0 (and friends) were introduced, which distinguish
formats with alpha from formats with padding. But before they were
introduced, the RGBA formats were flagged with AV_PIX_FMT_FLAG_ALPHA.
So the logical curse of action would be either marking PAL8 as
having alpha, or introducing a PALA8 format.
(Personally I'd say it would have been better to describe alpha
behavior with a separate enum. Then we could have distinguished
premultiplied and straight alpha, and could have kept the number of
pixfmts a but lower.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavformat/http: Refactor and fix additional leaks in get_cookies.
On Tue, 17 Apr 2018 16:37:13 -0700
rshaf...@tunein.com wrote:
> From: Richard Shaffer
>
> This refactors get_cookies to simplify some code paths, specifically for
> skipping logic in the while loop or exiting it. It also simplifies the logic
> for appending additional values to *cookies by replacing
> strlen/malloc/snprintf
> with one call av_asnprintf.
>
> This refactor fixes a bug where the cookie_params AVDictionary would get
> leaked
> if we failed to allocate a new buffer for writing to *cookies.
> ---
> libavformat/http.c | 64
> +++---
> 1 file changed, 27 insertions(+), 37 deletions(-)
>
> diff --git a/libavformat/http.c b/libavformat/http.c
> index b4a1919f24..183214c444 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -1015,7 +1015,8 @@ static int process_line(URLContext *h, char *line, int
> line_count,
> /**
> * Create a string containing cookie values for use as a HTTP cookie header
> * field value for a particular path and domain from the cookie values
> stored in
> - * the HTTP protocol context. The cookie string is stored in *cookies.
> + * the HTTP protocol context. The cookie string is stored in *cookies, and
> may
> + * be NULL if there are no valid cookies.
> *
> * @return a negative value if an error condition occurred, 0 otherwise
> */
> @@ -1025,15 +1026,19 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> // cookie strings will look like Set-Cookie header field values.
> Multiple
> // Set-Cookie fields will result in multiple values delimited by a
> newline
> int ret = 0;
> -char *cookie, *set_cookies = av_strdup(s->cookies), *next = set_cookies;
> -
> -if (!set_cookies) return AVERROR(EINVAL);
> +char *cookie, *set_cookies, *next;
>
> // destroy any cookies in the dictionary.
> av_dict_free(&s->cookie_dict);
>
> +if (!s->cookies)
> +return 0;
> +
> +if (!(next = set_cookies = av_strdup(s->cookies)))
> +return AVERROR(ENOMEM);
Nag: I think assignment in the if expression is really not necessary
here, even if we do it a lot in similar cases.
> +
> *cookies = NULL;
> -while ((cookie = av_strtok(next, "\n", &next))) {
> +while ((cookie = av_strtok(next, "\n", &next)) && !ret) {
> AVDictionary *cookie_params = NULL;
> AVDictionaryEntry *cookie_entry, *e;
>
> @@ -1043,23 +1048,19 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
>
> // continue on to the next cookie if this one cannot be parsed
> if (parse_set_cookie(cookie, &cookie_params))
> -continue;
> +goto skip_cookie;
>
> // if the cookie has no value, skip it
> cookie_entry = av_dict_get(cookie_params, "", NULL,
> AV_DICT_IGNORE_SUFFIX);
> -if (!cookie_entry || !cookie_entry->value) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (!cookie_entry || !cookie_entry->value)
> +goto skip_cookie;
>
> // if the cookie has expired, don't add it
> if ((e = av_dict_get(cookie_params, "expires", NULL, 0)) &&
> e->value) {
> struct tm tm_buf = {0};
> if (!parse_set_cookie_expiry_time(e->value, &tm_buf)) {
> -if (av_timegm(&tm_buf) < av_gettime() / 100) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (av_timegm(&tm_buf) < av_gettime() / 100)
> +goto skip_cookie;
> }
> }
>
> @@ -1067,42 +1068,31 @@ static int get_cookies(HTTPContext *s, char
> **cookies, const char *path,
> if ((e = av_dict_get(cookie_params, "domain", NULL, 0)) && e->value)
> {
> // find the offset comparison is on the min domain (b.com, not
> a.b.com)
> int domain_offset = strlen(domain) - strlen(e->value);
> -if (domain_offset < 0) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (domain_offset < 0)
> +goto skip_cookie;
>
> // match the cookie domain
> -if (av_strcasecmp(&domain[domain_offset], e->value)) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (av_strcasecmp(&domain[domain_offset], e->value))
> +goto skip_cookie;
> }
>
> // ensure this cookie matches the path
> e = av_dict_get(cookie_params, "path", NULL, 0);
> -if (!e || av_strncasecmp(path, e->value, strlen(e->value))) {
> -av_dict_free(&cookie_params);
> -continue;
> -}
> +if (!e || av_strncasecmp(path, e->value, strlen(e->value)))
> +goto skip_cookie;
>
> // cookie parameters match, so copy the value
>
Re: [FFmpeg-devel] [PATCH 3/4] avutil/pixdesc: add av_pix_fmt_desc_has_alpha()
On Thu, 19 Apr 2018 21:32:20 +0200
Marton Balint wrote:
> Signed-off-by: Marton Balint
> ---
> doc/APIchanges | 3 +++
> libavutil/pixdesc.h | 11 +++
> libavutil/version.h | 2 +-
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4f6ac2a031..2a0b6f057a 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>
> API changes, most recent first:
>
> +2018-04-xx - xx - lavu 56.16.100 - pixdesc.h
> + Add av_pix_fmt_desc_has_alpha().
> +
> 8< - FFmpeg 4.0 was cut here 8< -
>
> 2018-04-03 - d6fc031caf - lavu 56.13.100 - pixdesc.h
> diff --git a/libavutil/pixdesc.h b/libavutil/pixdesc.h
> index 1ab372782a..aef4313ccb 100644
> --- a/libavutil/pixdesc.h
> +++ b/libavutil/pixdesc.h
> @@ -430,4 +430,15 @@ int av_get_pix_fmt_loss(enum AVPixelFormat dst_pix_fmt,
> enum AVPixelFormat av_find_best_pix_fmt_of_2(enum AVPixelFormat
> dst_pix_fmt1, enum AVPixelFormat dst_pix_fmt2,
> enum AVPixelFormat src_pix_fmt,
> int has_alpha, int *loss_ptr);
>
> +/**
> + * Return true if a pixel format descriptor has alpha channel.
> + *
> + * @param desc the pixel format descriptor
> + * @return 1 if the pixel format descriptor has alpha, 0 otherwise.
> + */
> +static inline int av_pix_fmt_desc_has_alpha(const AVPixFmtDescriptor *desc)
> +{
> +return desc->nb_components == 2 || desc->nb_components == 4 ||
> (desc->flags & AV_PIX_FMT_FLAG_PAL);
> +}
Good, idea, but...
That doesn't seem very correct. Use AV_PIX_FMT_FLAG_ALPHA? (Although
PAL8 doesn't have it set, which is probably a bug. Or should have have
a separate PALA8 format?)
I also object to making it static inline, which not only can cause
problems in various cases, but also would make it part of the ABI (you
couldn't change the implementation without modifying the ABI,
essentially).
> +
> #endif /* AVUTIL_PIXDESC_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 387421775f..23567000a3 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 56
> -#define LIBAVUTIL_VERSION_MINOR 15
> +#define LIBAVUTIL_VERSION_MINOR 16
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] FFmpeg 3.5 / 4.0
On Thu, 19 Apr 2018 17:01:56 +0200 Nicolas George wrote: > James Almer (2018-04-19): > > It would have not been backwards compatible in such scenario to load at > > runtime an hypotetical 3.4.x lavf library with that change in an > > application that was built against 3.3.x or older. Regardless of 0 being > > defined as EOF or not in documentation, the behavior of one library > > would have not been the same as the other, at least as i said above, > > without the compat change you eventually committed. > > I did not remember the compat code was added afterwards. Anyway, it > excludes packet callbacks on purpose. > > The thing is, we could NOT fix the initial bug (EOF caused by empty UDP > packets, reported by an user). It just was not possible. Apart from > leaving the bug, there were two options to fix this: > > - break empty packet for all applications and all packets protocols, or > > - break packet callbacks returning 0 for EOF. - add a flag that controls the wanted behavior - return a special error code for 0 sized packets which users can treat as non-fatal (seems justified for such an obscure corner case) > Since empty packets are legitimate, and even used in a few multimedia > protocols, while callbacks returning 0 are using an undocumented and > illogical feature, there was little doubt about which one to break. There's still no way to use that via API (if I read retry_transfer_wrapper() and all the glue code correctly). ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
