Re: [FFmpeg-devel] [PATCH v5 00/25] Subtitle Filtering 2022
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Anton Khirnov
> Sent: Wednesday, August 31, 2022 3:40 AM
> To: FFmpeg development discussions and patches de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v5 00/25] Subtitle Filtering 2022
>
> Quoting Soft Works (2022-08-27 00:48:01)
> > 2. "There's no reason why this cannot be handled using the buffer
> > and data fields"
> >
> > I had explained the reasons and in conversation on IRC, Lynne was
> > no longer insisting on this AFAIR.
>
> I did not see this explanation, can you restate it here?
Sure. Let's look at the AVSubtitleArea struct first:
typedef struct AVSubtitleArea {
enum AVSubtitleType type;
int flags;
int x; ///< top left corner of area.
int y; ///< top left corner of area.
int w; ///< widthof area.
int h; ///< height of area.
int nb_colors; ///< number of colors in bitmap palette (@ref pal).
/**
* Buffers and line sizes for the bitmap of this subtitle.
*
* @{
*/
AVBufferRef *buf[AV_NUM_BUFFER_POINTERS];
int linesize[AV_NUM_BUFFER_POINTERS];
/**
* @}
*/
uint32_t pal[256]; ///< RGBA palette for the bitmap.
char *text;///< 0-terminated plain UTF-8 text
char *ass; ///< 0-terminated ASS/SSA compatible event line.
} AVSubtitleArea;
These structs are stored in AVFrame like this:
/**
* Number of items in the @ref subtitle_areas array.
*/
unsigned num_subtitle_areas;
/**
* Array of subtitle areas, may be empty.
*/
AVSubtitleArea **subtitle_areas;
The question was "why this cannot be handled using the buffer
and data fields?" - there are multiple reasons:
1. Area Count
In ASS subtitles it's not uncommon that animations are defined
through subtitle events (regular ass events). This can go as
far as that dust particles are flying around on the screen and
each particle has its own subtitle event/line which defines
how it flies from x to y and how fast, etc.
Not yet, but in a future update, the ass decoder should be
improved in a way that it doesn't emit an individual AVFrame
for each event line but bundles subsequent events together
when these would have the same start time and duration.
As a result, we can have AVFrames with dozens or even a hundred
AVSubtitleArea items in extreme cases.
The buffer and data fields are an array of fixed size (currently
8). Increasing to 16 or 32 would not help: there will still be
cases where this does not suffice.
2. What exactly should be stored in frame->buf[]?
Should we store the AVSubtitleArea structs as AVBuffer there?
Or should we only store data in those buffers? If yes, which
data? The subtitle bitmap probably. How about the subtitle
text - maybe and area has even both. And should we also
store the palette in some frame->buf[n]?
If yes, how to keep track of which data is stored in which
buffer?
Further, there's a documented convention that requires that
non-zero buffers are contiguous, i.e. there must not be
an empty buffer pointer between two other a non-empty buffer
pointers. This would require to re-arrange the buffers to
close any gap when some subtitle area data is removed, zeroed
out or has been converted to another format.
Closing such gap also require to update any mapping information
("which buffer is related to which area?")
That's a lot of tedious work and every API user (or codec/filter
developer) would need to do it in the right way.
3. AVFrame Code Logic Mistmatch
A subtitle frame can have 0 to N areas, which means that a
frame without any area is still valid. Opposed to that, existing
(code all over the place in ffmpeg) is treating an AVFrame
as invalid when the first data buffer is empty,
i.e. frame->buf[0] != NULL
To accommodate to this situation, subtitle frames are always
creating a 1-byte buffer for buf[0].
When we would want subtitle data to be stored in those buffers,
every developer would need to be aware about the requirement
to have that dummy buffer in the first array element. When something
is to be stored, the dummy buffer would need to be freed first
before storing the actual data.
And when the last buffer gets deleted, API users would need to
know to create a new dummy buffer.
That fixed size array might be a good fit for image data, but
it's not for subtitle data.
The approach in my patchset is simple, intuitive and everybody
can easily work with it without needing any special knowledge:
unsigned num_subtitle_areas;
AVSubtitleArea **subtitle_areas;
IMHO, this is the most suitable pattern for this situation.
> If you claim the other points were addressed I will look at the
> patches
> again.
Oh cool, that would be great!
Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To
Re: [FFmpeg-devel] [PATCH 16/16] avutil/fifo: Properly deprecate av_tempfile()
Anton Khirnov: > Quoting Andreas Rheinhardt (2022-08-29 01:34:48) >> Subject: Re: [FFmpeg-devel] [PATCH 16/16] avutil/fifo: Properly deprecate >> av_tempfile() > > you mean file > Thanks for noticing, will fix. (It is btw not the only bug in this patchset: #15 got the header inclusion guards wrong (there is space before the #define and I have no clue where it comes from).) - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 16/16] avutil/fifo: Properly deprecate av_tempfile()
Quoting Andreas Rheinhardt (2022-08-29 01:34:48) > Subject: Re: [FFmpeg-devel] [PATCH 16/16] avutil/fifo: Properly deprecate > av_tempfile() you mean file -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Leo Izen: > > On 8/30/22 15:37, Nicolas George wrote: >> Leo Izen (12022-08-30): >>> Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = >>> foo()? >>> Most other APIs return pointers to structs, rather than structs >>> themselves >>> (see: av_packet_alloc). Using a pointer would prevent us from having >>> sizeof(AVWriter) as part of the ABI, as was done with AVPacket. >> >> Yes: to return a pointer, you need somewhere to store the structure. One >> of the point of AVWriter is that you can store it on the stack to avoid >> dynamic allocations when the string is short enough. >> >> Note that AVWriter is exactly two pointers. It will always be two >> pointers, and all the objects that I intend to introduce later will >> always be two pointers: one const pointer for the methods, one pointer >> for the object itself. >> >> This design is essential to the features I promised for AVWriter and for >> later. >> > > I don't see how you are planning on avoiding dynamic allocations by > stack-allocating AVWriter when AVWriter itself only contains two pointers. > > I also don't see how this design is essential to the features you > promised in a way that can't be done by just not making sizeof(AVWriter) > part of the ABI and returning pointers to a struct. > He is not only stack-allocating AVWriter, he also intends to stack-allocate the actual writers like AVDynbufWriter, AVBufWriter (AVWriter is just a wrapper around the underlying writers). This means that no allocations need to be performed for AVBufWriter at all (and due to the inherent small-string optimization of an AVBPrint, it can also be avoided for AVDynbufWriter in lots of cases). If you return pointers to an AVWriter struct, you need to allocate this struct somewhere, which means that your init/av_dynbuf_writer_wrap has an allocation that can fail and therefore needs to be checked; and the struct needs to be freed lateron. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On 8/30/22 15:37, Nicolas George wrote: Leo Izen (12022-08-30): Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = foo()? Most other APIs return pointers to structs, rather than structs themselves (see: av_packet_alloc). Using a pointer would prevent us from having sizeof(AVWriter) as part of the ABI, as was done with AVPacket. Yes: to return a pointer, you need somewhere to store the structure. One of the point of AVWriter is that you can store it on the stack to avoid dynamic allocations when the string is short enough. Note that AVWriter is exactly two pointers. It will always be two pointers, and all the objects that I intend to introduce later will always be two pointers: one const pointer for the methods, one pointer for the object itself. This design is essential to the features I promised for AVWriter and for later. I don't see how you are planning on avoiding dynamic allocations by stack-allocating AVWriter when AVWriter itself only contains two pointers. I also don't see how this design is essential to the features you promised in a way that can't be done by just not making sizeof(AVWriter) part of the ABI and returning pointers to a struct. - Leo Izen (thebombzen) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 17/18] lavf/dv: do not update AVCodecParameters.sample_rate while demuxing
Quoting Andreas Rheinhardt (2022-08-24 16:20:35) > Anton Khirnov: > > Demuxers are not allowed to do this and few callers, if any, will handle > > this correctly. Send the AV_SIDE_DATA_PARAM_CHANGE_SAMPLE_RATE side data > > instead. > > 1. One of the callers which did not handle this correctly is the mov > demuxer: A sample rate update would not propagate to the user, because > it uses a separate AVFormatContext for it. This should be mentioned in > the commit message. (Btw: I don't see anything that guarantees that the > samplerate and timebase of the inner demuxer and the sample rate and > time base reported to the user (presumably taken from mov structures) > coincide. Given that dv_fctx and dv_demux are part of MOVContext, they > would also leak if it would be attempted to allocate them multiple > times. Do you see anything that guarantees that they will not be > allocated multiple times?) > 2. In case of ff_add_param_change() failure, users will just interpret > that as "no more packets available", won't they? yes, I think so > This might be wrong, but it is not problematic. I agree > 3. Have you tested this with a sample with actual parameter changes? Yes, I made a sample by concatenating two dv files. The side data is exported correctly, but the PCM decoder fails because it is not marked with AV_CODEC_CAP_PARAM_CHANGE. Patches welcome. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH initialization
Lukas Fellechner:
> This patch adds an "init-threads" option, specifying the max
> number of threads to use. Multiple worker threads are spun up
> to massively bring down init times.
> ---
> libavformat/dashdec.c | 351 +-
> 1 file changed, 350 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index e82da45e43..20f2557ea3 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -24,6 +24,7 @@
> #include "libavutil/opt.h"
> #include "libavutil/time.h"
> #include "libavutil/parseutils.h"
> +#include "libavutil/thread.h"
> #include "internal.h"
> #include "avio_internal.h"
> #include "dash.h"
> @@ -152,6 +153,8 @@ typedef struct DASHContext {
> int max_url_size;
> char *cenc_decryption_key;
>
> +int init_threads;
> +
> /* Flags for init section*/
> int is_init_section_common_video;
> int is_init_section_common_audio;
> @@ -2033,6 +2036,331 @@ static void move_metadata(AVStream *st, const char
> *key, char **value)
> }
> }
>
> +#if HAVE_THREADS
> +
> +struct work_pool_data
> +{
> +AVFormatContext *ctx;
> +struct representation *pls;
> +struct representation *common_pls;
> +pthread_mutex_t *common_mutex;
> +pthread_cond_t *common_condition;
> +int is_common;
> +int is_started;
> +int result;
> +};
> +
> +struct thread_data
This is against our naming conventions: CamelCase for struct tags and
typedefs, lowercase names with underscore for variable names.
> +{
> +pthread_t thread;
> +pthread_mutex_t *mutex;
> +struct work_pool_data *work_pool;
> +int work_pool_size;
> +int is_started;
> +int has_error;
> +};
> +
> +static void *worker_thread(void *ptr)
> +{
> +int ret = 0;
> +int i;
> +struct thread_data *thread_data = (struct thread_data*)ptr;
> +struct work_pool_data *work_pool = NULL;
> +struct work_pool_data *data = NULL;
> +for (;;) {
> +
> +// get next work item unless there was an error
> +pthread_mutex_lock(thread_data->mutex);
> +data = NULL;
> +if (!thread_data->has_error) {
> +work_pool = thread_data->work_pool;
> +for (i = 0; i < thread_data->work_pool_size; i++) {
> +if (!work_pool->is_started) {
> +data = work_pool;
> +data->is_started = 1;
> +break;
> +}
> +work_pool++;
> +}
> +}
> +pthread_mutex_unlock(thread_data->mutex);
> +
> +if (!data) {
> +// no more work to do
> +return NULL;
> +}
> +
> +// if we are common section provider, init and signal
> +if (data->is_common) {
> +data->pls->parent = data->ctx;
> +ret = update_init_section(data->pls);
> +if (ret < 0) {
> +pthread_cond_signal(data->common_condition);
> +goto end;
> +}
> +else
> +ret = AVERROR(pthread_cond_signal(data->common_condition));
> +}
> +
> +// if we depend on common section provider, wait for signal and copy
> +if (data->common_pls) {
> +ret = AVERROR(pthread_cond_wait(data->common_condition,
> data->common_mutex));
> +if (ret < 0)
> +goto end;
> +
> +if (!data->common_pls->init_sec_buf) {
> +goto end;
> +ret = AVERROR(EFAULT);
> +}
> +
> +ret = copy_init_section(data->pls, data->common_pls);
> +if (ret < 0)
> +goto end;
> +}
> +
> +ret = begin_open_demux_for_component(data->ctx, data->pls);
> +if (ret < 0)
> +goto end;
> +
> +end:
> +data->result = ret;
> +
> +// notify error to other threads and exit
> +if (ret < 0) {
> +pthread_mutex_lock(thread_data->mutex);
> +thread_data->has_error = 1;
> +pthread_mutex_unlock(thread_data->mutex);
> +return NULL;
> +}
> +}
> +
> +
> +return NULL;
> +}
> +
> +static void create_work_pool_data(AVFormatContext *ctx, int stream_index,
> +struct representation *pls, struct representation *common_pls,
> +struct work_pool_data *init_data, pthread_mutex_t *common_mutex,
> +pthread_cond_t *common_condition)
> +{
> +init_data->ctx = ctx;
> +init_data->pls = pls;
> +init_data->pls->stream_index = stream_index;
> +init_data->common_condition = common_condition;
> +init_data->common_mutex = common_mutex;
> +init_data->result = -1;
> +
> +if (pls == common_pls) {
> +init_data->is_common = 1;
> +}
> +else if (common_pls) {
> +init_data->common_pls = common_pls;
> +}
> +}
> +
> +static int start_thread(struct thread_data *thread_data,
> +struct work_pool_data *work_pool, int
[FFmpeg-devel] [PATCH] lavf/dv: return a meaningful error code from avpriv_dv_produce_packet()
---
libavformat/dv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/dv.c b/libavformat/dv.c
index f88fe62349..c888111789 100644
--- a/libavformat/dv.c
+++ b/libavformat/dv.c
@@ -404,7 +404,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket
*pkt,
if (buf_size < DV_PROFILE_BYTES ||
!(c->sys = av_dv_frame_profile(c->sys, buf, buf_size)) ||
buf_size < c->sys->frame_size) {
-return -1; /* Broken frame, or not enough data */
+return AVERROR_INVALIDDATA;
}
/* Queueing audio packet */
--
2.35.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] lavf/mov: avoid leaks with multiple dv-audio streams
---
libavformat/mov.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a2b429e52f..f433746192 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -2430,6 +2430,11 @@ static int mov_finalize_stsd_codec(MOVContext *c,
AVIOContext *pb,
switch (st->codecpar->codec_id) {
#if CONFIG_DV_DEMUXER
case AV_CODEC_ID_DVAUDIO:
+if (c->dv_fctx) {
+avpriv_request_sample(c->fc, "multiple DV audio streams");
+return AVERROR(ENOSYS);
+}
+
c->dv_fctx = avformat_alloc_context();
if (!c->dv_fctx) {
av_log(c->fc, AV_LOG_ERROR, "dv demux context alloc error\n");
--
2.35.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 12/18] lavf/dv: make returning the video packet optional
Quoting Andreas Rheinhardt (2022-08-24 18:03:05)
> Anton Khirnov:
> > The mov demuxer only returns DV audio, video packets are discarded.
> >
> > It first reads the data to be parsed into a packet. Then both this
> > packet and the pointer to its data are passed together to
> > avpriv_dv_produce_packet(), which parses the data and partially
> > overwrites the packet. This is confusing and potentially dangerous, so
> > just pass NULL and avoid pointless packet modification.
> > ---
> > libavformat/dv.c | 19 +++
> > libavformat/mov.c | 2 +-
> > 2 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavformat/dv.c b/libavformat/dv.c
> > index 303cecf9bb..f88fe62349 100644
> > --- a/libavformat/dv.c
> > +++ b/libavformat/dv.c
> > @@ -430,14 +430,17 @@ int avpriv_dv_produce_packet(DVDemuxContext *c,
> > AVPacket *pkt,
> > }
> > }
> >
> > -/* Now it's time to return video packet */
> > -size = dv_extract_video_info(c, buf);
> > -pkt->data = buf;
> > -pkt->pos = pos;
> > -pkt->size = size;
> > -pkt->flags |= AV_PKT_FLAG_KEY;
> > -pkt->stream_index = c->vst->index;
> > -pkt->pts = c->frames;
> > +/* return the video packet, if the caller wants it */
> > +if (pkt) {
> > +size = dv_extract_video_info(c, buf);
> > +
> > +pkt->data = buf;
> > +pkt->pos = pos;
> > +pkt->size = size;
> > +pkt->flags |= AV_PKT_FLAG_KEY;
> > +pkt->stream_index = c->vst->index;
> > +pkt->pts = c->frames;
> > +}
> >
> > c->frames++;
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 1d8c5b2904..a2b429e52f 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -8768,7 +8768,7 @@ static int mov_read_packet(AVFormatContext *s,
> > AVPacket *pkt)
> > }
> > #if CONFIG_DV_DEMUXER
> > if (mov->dv_demux && sc->dv_audio_container) {
> > -ret = avpriv_dv_produce_packet(mov->dv_demux, pkt, pkt->data,
> > pkt->size, pkt->pos);
> > +ret = avpriv_dv_produce_packet(mov->dv_demux, NULL, pkt->data,
> > pkt->size, pkt->pos);
> > av_packet_unref(pkt);
> > if (ret < 0)
> > return ret;
>
> 1. LGTM.
> 2. The way mov handles dv audio is very broken:
> a) I don't see anything that guarantees that the
> samplerate of the inner demuxer and the sample rate reported to the user
> (presumably taken from mov structures) coincide.
> b) dv_fctx and dv_demux are part of MOVContext, not MOVStreamContext. If
> there were multiple MOVStreams with dv-audio, the older dv_fctx and
> dv_demux would leak. Do you see anything that guarantees that they will
> only be at most one MOVStream for dv-audio?
Not that I can see. I'd say the best thing to do here is fail when a
dvaudio stream already exists - trying to support it without having any
valid samples seems pointless. I'll send a patch.
> c) The former would be easily fixable by moving the fields to
> MOVStreamContext. But there is actually another bug: The underlying dv
> stream can contain multiple audio streams in this one MOVStream and only
> the first of these is forwarded.
> https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4671/dir1.tar.bz2
> contains a sample where this actually occurs: The DV stream has two
> stereo tracks, only the first of which is forwarded (use -enable_drefs 1
> to be able to play it; the non-forwarded seems to be silent in this
> case). Are these actually independent audios or are these stereo pairs
> part of a single streams with more than two channels?
Cannot answer this. I would ignore this until and unless someone
actually comes complaining about this.
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/3] lavf/dashdec: Prepare DASH decoder for multithreading
Lukas Fellechner 于2022年8月24日周三 03:04写道：
>
> For adding multithreading to the DASH decoder initialization,
> the open_demux_for_component() method must be split up into two parts:
>
> begin_open_demux_for_component(): Opens the stream and does probing
> and format detection. This can be run in parallel.
>
> end_open_demux_for_component(): Creates the AVStreams and adds
> them to the common parent AVFormatContext. This method must always be
> run synchronously, after all threads are finished.
> ---
> libavformat/dashdec.c | 42 ++
> 1 file changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 63bf7e96a5..e82da45e43 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -1918,10 +1918,9 @@ fail:
> return ret;
> }
>
> -static int open_demux_for_component(AVFormatContext *s, struct
> representation *pls)
> +static int begin_open_demux_for_component(AVFormatContext *s, struct
> representation *pls)
> {
> int ret = 0;
> -int i;
>
> pls->parent = s;
> pls->cur_seq_no = calc_cur_seg_no(s, pls);
> @@ -1931,9 +1930,15 @@ static int open_demux_for_component(AVFormatContext
> *s, struct representation *p
> }
>
> ret = reopen_demux_for_component(s, pls);
> -if (ret < 0) {
> -goto fail;
> -}
> +
> +return ret;
> +}
> +
> +static int end_open_demux_for_component(AVFormatContext *s, struct
> representation *pls)
> +{
> +int ret = 0;
> +int i;
> +
> for (i = 0; i < pls->ctx->nb_streams; i++) {
> AVStream *st = avformat_new_stream(s, NULL);
> AVStream *ist = pls->ctx->streams[i];
> @@ -1965,6 +1970,19 @@ fail:
> return ret;
> }
>
> +static int open_demux_for_component(AVFormatContext* s, struct
> representation* pls)
> +{
> +int ret = 0;
> +
> +ret = begin_open_demux_for_component(s, pls);
> +if (ret < 0)
> +return ret;
> +
> +ret = end_open_demux_for_component(s, pls);
> +
> +return ret;
> +}
> +
> static int is_common_init_section_exist(struct representation **pls, int
> n_pls)
> {
> struct fragment *first_init_section = pls[0]->init_section;
> @@ -2040,9 +2058,15 @@ static int dash_read_header(AVFormatContext *s)
> av_dict_set(>avio_opts, "seekable", "0", 0);
> }
>
> -if(c->n_videos)
> +if (c->n_videos)
> c->is_init_section_common_video =
> is_common_init_section_exist(c->videos, c->n_videos);
>
> +if (c->n_audios)
> +c->is_init_section_common_audio =
> is_common_init_section_exist(c->audios, c->n_audios);
> +
> +if (c->n_subtitles)
> +c->is_init_section_common_subtitle =
> is_common_init_section_exist(c->subtitles, c->n_subtitles);
> +
> /* Open the demuxer for video and audio components if available */
> for (i = 0; i < c->n_videos; i++) {
> rep = c->videos[i];
> @@ -2059,9 +2083,6 @@ static int dash_read_header(AVFormatContext *s)
> ++stream_index;
> }
>
> -if(c->n_audios)
> -c->is_init_section_common_audio =
> is_common_init_section_exist(c->audios, c->n_audios);
> -
> for (i = 0; i < c->n_audios; i++) {
> rep = c->audios[i];
> if (i > 0 && c->is_init_section_common_audio) {
> @@ -2077,9 +2098,6 @@ static int dash_read_header(AVFormatContext *s)
> ++stream_index;
> }
>
> -if (c->n_subtitles)
> -c->is_init_section_common_subtitle =
> is_common_init_section_exist(c->subtitles, c->n_subtitles);
> -
> for (i = 0; i < c->n_subtitles; i++) {
> rep = c->subtitles[i];
> if (i > 0 && c->is_init_section_common_subtitle) {
> --
> 2.31.1.windows.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Patchset looks ok and test passed here. Any comments?
Thanks
Steven
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 00/25] Subtitle Filtering 2022
Quoting Soft Works (2022-08-27 00:48:01) > 2. "There's no reason why this cannot be handled using the buffer > and data fields" > > I had explained the reasons and in conversation on IRC, Lynne was > no longer insisting on this AFAIR. I did not see this explanation, can you restate it here? If you claim the other points were addressed I will look at the patches again. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/decode: Warp get_hw_config function
On Tue, 2022-08-23 at 16:19 +0800, Fei Wang wrote:
> From: Linjie Fu
>
> Wrap the procedure of getting the hardware config from a pixel format
> into a function.
>
> Signed-off-by: Linjie Fu
> Signed-off-by: Fei Wang
> ---
> libavcodec/decode.c | 31 +++
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 75373989c6..3b69426c09 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1156,6 +1156,24 @@ static void hwaccel_uninit(AVCodecContext
> *avctx)
> av_buffer_unref(>hw_frames_ctx);
> }
>
> +static const AVCodecHWConfigInternal *get_hw_config(AVCodecContext
> *avctx, enum AVPixelFormat fmt)
> +{
> +const AVCodecHWConfigInternal *hw_config;
> +
> +if (!ffcodec(avctx->codec)->hw_configs)
> +return NULL;
> +
> +for (int i = 0;; i++) {
> +hw_config = ffcodec(avctx->codec)->hw_configs[i];
> +if (!hw_config)
> +return NULL;
> +if (hw_config->public.pix_fmt == fmt)
> +return hw_config;
> +}
> +
> +return NULL;
> +}
> +
> int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat
> *fmt)
> {
> const AVPixFmtDescriptor *desc;
> @@ -1213,18 +1231,7 @@ int ff_get_format(AVCodecContext *avctx, const
> enum AVPixelFormat *fmt)
> break;
> }
>
> -if (ffcodec(avctx->codec)->hw_configs) {
> -for (i = 0;; i++) {
> -hw_config = ffcodec(avctx->codec)->hw_configs[i];
> -if (!hw_config)
> -break;
> -if (hw_config->public.pix_fmt == user_choice)
> -break;
> -}
> -} else {
> -hw_config = NULL;
> -}
> -
> +hw_config = get_hw_config(avctx, user_choice);
> if (!hw_config) {
> // No config available, so no extra setup required.
> ret = user_choice;
Ping, any more comments on V3?
Thanks
Fei
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3 v2] avformat/avisynth: reindent
Signed-off-by: Stephen Hutchinson
---
libavformat/avisynth.c | 348 -
1 file changed, 174 insertions(+), 174 deletions(-)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 7bb2977383..b426ac343e 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -533,235 +533,235 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
/* Field order */
if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER) {
-if(avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") ==
AVS_PROPTYPE_UNSET) {
-st->codecpar->field_order = AV_FIELD_UNKNOWN;
-} else {
-switch (avs_library.avs_prop_get_int(avs->env, avsmap,
"_FieldBased", 0, )) {
-case 0:
-st->codecpar->field_order = AV_FIELD_PROGRESSIVE;
-break;
-case 1:
-st->codecpar->field_order = AV_FIELD_BB;
-break;
-case 2:
-st->codecpar->field_order = AV_FIELD_TT;
-break;
-default:
+if(avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased")
== AVS_PROPTYPE_UNSET) {
st->codecpar->field_order = AV_FIELD_UNKNOWN;
+} else {
+switch (avs_library.avs_prop_get_int(avs->env, avsmap,
"_FieldBased", 0, )) {
+case 0:
+st->codecpar->field_order = AV_FIELD_PROGRESSIVE;
+break;
+case 1:
+st->codecpar->field_order = AV_FIELD_BB;
+break;
+case 2:
+st->codecpar->field_order = AV_FIELD_TT;
+break;
+default:
+st->codecpar->field_order = AV_FIELD_UNKNOWN;
+}
}
}
-}
/* Color Range */
if(avs->flags & AVISYNTH_FRAMEPROP_RANGE) {
-if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") ==
AVS_PROPTYPE_UNSET) {
-st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
-} else {
-switch (avs_library.avs_prop_get_int(avs->env, avsmap,
"_ColorRange", 0, )) {
-case 0:
-st->codecpar->color_range = AVCOL_RANGE_JPEG;
-break;
-case 1:
-st->codecpar->color_range = AVCOL_RANGE_MPEG;
-break;
-default:
+if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange")
== AVS_PROPTYPE_UNSET) {
st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
+} else {
+switch (avs_library.avs_prop_get_int(avs->env, avsmap,
"_ColorRange", 0, )) {
+case 0:
+st->codecpar->color_range = AVCOL_RANGE_JPEG;
+break;
+case 1:
+st->codecpar->color_range = AVCOL_RANGE_MPEG;
+break;
+default:
+st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
+}
}
}
-}
/* Color Primaries */
if(avs->flags & AVISYNTH_FRAMEPROP_PRIMARIES) {
-switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries",
0, )) {
-case 1:
-st->codecpar->color_primaries = AVCOL_PRI_BT709;
-break;
-case 2:
-st->codecpar->color_primaries = AVCOL_PRI_UNSPECIFIED;
-break;
-case 4:
-st->codecpar->color_primaries = AVCOL_PRI_BT470M;
-break;
-case 5:
-st->codecpar->color_primaries = AVCOL_PRI_BT470BG;
-break;
-case 6:
-st->codecpar->color_primaries = AVCOL_PRI_SMPTE170M;
-break;
-case 7:
-st->codecpar->color_primaries = AVCOL_PRI_SMPTE240M;
-break;
-case 8:
-st->codecpar->color_primaries = AVCOL_PRI_FILM;
-break;
-case 9:
-st->codecpar->color_primaries = AVCOL_PRI_BT2020;
-break;
-case 10:
-st->codecpar->color_primaries = AVCOL_PRI_SMPTE428;
-break;
-case 11:
-st->codecpar->color_primaries = AVCOL_PRI_SMPTE431;
-break;
-case 12:
-st->codecpar->color_primaries = AVCOL_PRI_SMPTE432;
-break;
-case 22:
-st->codecpar->color_primaries = AVCOL_PRI_EBU3213;
-break;
-default:
-st->codecpar->color_primaries = AVCOL_PRI_UNSPECIFIED;
-}
+switch (avs_library.avs_prop_get_int(avs->env, avsmap,
"_Primaries", 0, )) {
+case 1:
+st->codecpar->color_primaries = AVCOL_PRI_BT709;
+break;
+case 2:
+st->codecpar->color_primaries = AVCOL_PRI_UNSPECIFIED;
+
[FFmpeg-devel] [PATCH 2/3 v2] avformat/avisynth: implement avisynth_flags option
Signed-off-by: Stephen Hutchinson
---
libavformat/avisynth.c | 52 ++
1 file changed, 52 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index d978e6ec40..7bb2977383 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -21,6 +21,7 @@
#include "libavutil/attributes.h"
#include "libavutil/internal.h"
+#include "libavutil/opt.h"
#include "libavcodec/internal.h"
@@ -85,7 +86,18 @@ typedef struct AviSynthLibrary {
#undef AVSC_DECLARE_FUNC
} AviSynthLibrary;
+typedef enum AviSynthFlags {
+AVISYNTH_FRAMEPROP_FIELD_ORDER = (1 << 0),
+AVISYNTH_FRAMEPROP_RANGE = (1 << 1),
+AVISYNTH_FRAMEPROP_PRIMARIES = (1 << 2),
+AVISYNTH_FRAMEPROP_TRANSFER = (1 << 3),
+AVISYNTH_FRAMEPROP_MATRIX = (1 << 4),
+AVISYNTH_FRAMEPROP_CHROMA_LOCATION = (1 << 5),
+AVISYNTH_FRAMEPROP_SAR = (1 << 6),
+} AviSynthFlags;
+
typedef struct AviSynthContext {
+const AVClass *class;
AVS_ScriptEnvironment *env;
AVS_Clip *clip;
const AVS_VideoInfo *vi;
@@ -100,6 +112,8 @@ typedef struct AviSynthContext {
int error;
+uint32_t flags;
+
/* Linked list pointers. */
struct AviSynthContext *next;
} AviSynthContext;
@@ -518,6 +532,7 @@ static int avisynth_create_stream_video(AVFormatContext *s,
AVStream *st)
avsmap = avs_library.avs_get_frame_props_ro(avs->env, frame);
/* Field order */
+if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER) {
if(avs_library.avs_prop_get_type(avs->env, avsmap, "_FieldBased") ==
AVS_PROPTYPE_UNSET) {
st->codecpar->field_order = AV_FIELD_UNKNOWN;
} else {
@@ -535,8 +550,10 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->codecpar->field_order = AV_FIELD_UNKNOWN;
}
}
+}
/* Color Range */
+if(avs->flags & AVISYNTH_FRAMEPROP_RANGE) {
if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ColorRange") ==
AVS_PROPTYPE_UNSET) {
st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
} else {
@@ -551,8 +568,10 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->codecpar->color_range = AVCOL_RANGE_UNSPECIFIED;
}
}
+}
/* Color Primaries */
+if(avs->flags & AVISYNTH_FRAMEPROP_PRIMARIES) {
switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Primaries",
0, )) {
case 1:
st->codecpar->color_primaries = AVCOL_PRI_BT709;
@@ -593,8 +612,10 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
default:
st->codecpar->color_primaries = AVCOL_PRI_UNSPECIFIED;
}
+}
/* Color Transfer Characteristics */
+if(avs->flags & AVISYNTH_FRAMEPROP_TRANSFER) {
switch (avs_library.avs_prop_get_int(avs->env, avsmap, "_Transfer", 0,
)) {
case 1:
st->codecpar->color_trc = AVCOL_TRC_BT709;
@@ -650,8 +671,10 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
default:
st->codecpar->color_trc = AVCOL_TRC_UNSPECIFIED;
}
+}
/* Matrix coefficients */
+if(avs->flags & AVISYNTH_FRAMEPROP_MATRIX) {
if(avs_library.avs_prop_get_type(avs->env, avsmap, "_Matrix") ==
AVS_PROPTYPE_UNSET) {
st->codecpar->color_space = AVCOL_SPC_UNSPECIFIED;
} else {
@@ -702,8 +725,10 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->codecpar->color_space = AVCOL_SPC_UNSPECIFIED;
}
}
+}
/* Chroma Location */
+if(avs->flags & AVISYNTH_FRAMEPROP_CHROMA_LOCATION) {
if(avs_library.avs_prop_get_type(avs->env, avsmap, "_ChromaLocation")
== AVS_PROPTYPE_UNSET) {
st->codecpar->chroma_location = AVCHROMA_LOC_UNSPECIFIED;
} else {
@@ -730,11 +755,14 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->codecpar->chroma_location = AVCHROMA_LOC_UNSPECIFIED;
}
}
+}
/* Sample aspect ratio */
+if(avs->flags & AVISYNTH_FRAMEPROP_SAR) {
sar_num = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0,
);
sar_den = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0,
);
st->sample_aspect_ratio = (AVRational){ sar_num, sar_den };
+}
avs_library.avs_release_video_frame(frame);
} else {
@@ -1140,6 +1168,29 @@ static int avisynth_read_seek(AVFormatContext *s, int
stream_index,
return 0;
}
+#define AVISYNTH_FRAMEPROP_DEFAULT AVISYNTH_FRAMEPROP_FIELD_ORDER |
AVISYNTH_FRAMEPROP_RANGE | \
+ AVISYNTH_FRAMEPROP_PRIMARIES |
AVISYNTH_FRAMEPROP_TRANSFER | \
+
[FFmpeg-devel] [PATCH 1/3 v2] avformat/avisynth: read _SARNum/_SARDen from frame properties
Initialized to 1:1, but if the script sets these properties, it
will be set to those instead (0:0 disables it, apparently).
Signed-off-by: Stephen Hutchinson
---
libavformat/avisynth.c | 8
1 file changed, 8 insertions(+)
diff --git a/libavformat/avisynth.c b/libavformat/avisynth.c
index 3d9fa2be50..d978e6ec40 100644
--- a/libavformat/avisynth.c
+++ b/libavformat/avisynth.c
@@ -251,6 +251,8 @@ static int avisynth_create_stream_video(AVFormatContext *s,
AVStream *st)
AVS_VideoFrame *frame;
int error;
int planar = 0; // 0: packed, 1: YUV, 2: Y8, 3: Planar RGB, 4: YUVA, 5:
Planar RGBA
+int sar_num = 1;
+int sar_den = 1;
st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
st->codecpar->codec_id = AV_CODEC_ID_RAWVIDEO;
@@ -728,6 +730,12 @@ static int avisynth_create_stream_video(AVFormatContext
*s, AVStream *st)
st->codecpar->chroma_location = AVCHROMA_LOC_UNSPECIFIED;
}
}
+
+/* Sample aspect ratio */
+sar_num = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARNum", 0,
);
+sar_den = avs_library.avs_prop_get_int(avs->env, avsmap, "_SARDen", 0,
);
+st->sample_aspect_ratio = (AVRational){ sar_num, sar_den };
+
avs_library.avs_release_video_frame(frame);
} else {
st->codecpar->field_order = AV_FIELD_UNKNOWN;
--
2.34.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 0/3 v2] avisynth: add user-selectable flags
The reading of frame properties from the script can now be toggled on and off per-property or as a whole, using the avisynth_flags option. The ability to read _SARNum/_SARDen properties has been added, but is kept off by default because it poses more of a risk of a user accidentally getting it wrong than the already existing properties do, which is what prompted adding the ability to switch frame property reading on and off. Stephen Hutchinson (3): avformat/avisynth: read _SARNum/_SARDen from frame properties avformat/avisynth: implement avisynth_flags option avformat/avisynth: reindent libavformat/avisynth.c | 386 - 1 file changed, 223 insertions(+), 163 deletions(-) -- 2.34.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/9] avformat/avisynth: add read_frameprops option
On 8/30/22 7:17 PM, Andreas Rheinhardt wrote:
{ "avisynth_flags", "set flags related to reading frame properties from
script (AviSynth+ v3.7.1 or higher)", OFFSET(flags), AV_OPT_TYPE_FLAGS,
{.i64 = 1}, 0, 1, AV_OPT_FLAG_DECODING_PARAM, "flags" },
This is wrong. It should be
{ "avisynth_flags", "set flags related to reading frame properties from
script (AviSynth+ v3.7.1 or higher)", OFFSET(flags), AV_OPT_TYPE_FLAGS,
{.i64 = AVISYNTH_FRAMEPROP_FIELD_ORDER | AVISYNTH_FRAMEPROP_RANGE |
AVISYNTH_FRAMEPROP_PRIMARIES | AVISYNTH_FRAMEPROP_TRANSFER |
AVISYNTH_FRAMEPROP_MATRIX | AVISYNTH_FRAMEPROP_CHROMA_LOCATION}, 0,
INT_MAX, AV_OPT_FLAG_DECODING_PARAM, "flags" }
The default option should be removed. Users can then set the options via
avisynth_flags=+sar-range or via avisynth_flags=matrix or however they wish.
The AVISYNTH_FRAMEPROP_DEFAULT should also be removed (at least, it
should not be part of the bitfield, but you can of course add a define
(or an enum value) equivalent to the default value I used above and you
can use that for the default value above); to know whether field order
should be exported, you simply query via "if (avs->flags &
AVISYNTH_FRAMEPROP_FIELD_ORDER)". For this it is of course important
that the default value is a combination of other bits of the bitfield
and not a bit of its own.
I don't know why I didn't think to use it directly in the avisynth_flags
line. Thanks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/asfdec_o: limit recursion depth in asf_read_unknown()
The threshold of 5 is arbitrary, both smaller and larger should work fine
Fixes: Stack overflow
Fixes:
50603/clusterfuzz-testcase-minimized-ffmpeg_dem_ASF_O_fuzzer-6049302564175872
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavformat/asfdec_o.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libavformat/asfdec_o.c b/libavformat/asfdec_o.c
index 907be6de04f..48b7d17322d 100644
--- a/libavformat/asfdec_o.c
+++ b/libavformat/asfdec_o.c
@@ -109,6 +109,7 @@ typedef struct ASFContext {
int64_t data_offset;
int64_t first_packet_offset; // packet offset
int64_t unknown_offset; // for top level header objects or subobjects
without specified behavior
+int in_asf_read_unknown;
// ASF file must not contain more than 128 streams according to the
specification
ASFStream *asf_st[ASF_MAX_STREAMS];
@@ -173,7 +174,7 @@ static int asf_read_unknown(AVFormatContext *s, const
GUIDParseTable *g)
uint64_t size = avio_rl64(pb);
int ret;
-if (size > INT64_MAX)
+if (size > INT64_MAX || asf->in_asf_read_unknown > 5)
return AVERROR_INVALIDDATA;
if (asf->is_header)
@@ -182,8 +183,11 @@ static int asf_read_unknown(AVFormatContext *s, const
GUIDParseTable *g)
if (!g->is_subobject) {
if (!(ret = strcmp(g->name, "Header Extension")))
avio_skip(pb, 22); // skip reserved fields and Data Size
-if ((ret = detect_unknown_subobject(s, asf->unknown_offset,
-asf->unknown_size)) < 0)
+asf->in_asf_read_unknown ++;
+ret = detect_unknown_subobject(s, asf->unknown_offset,
+asf->unknown_size);
+asf->in_asf_read_unknown --;
+if (ret < 0)
return ret;
} else {
if (size < 24) {
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/9] avformat/avisynth: add read_frameprops option
Stephen Hutchinson:
> On 8/28/22 8:41 PM, Andreas Rheinhardt wrote:
>> This will make frameprops a global on-off which overrides everything
>> else even if some of the "else" stuff has been enabled explicitly. Worse
>> yet, if you want to disable everything except exactly one field, you
>> have to leave frameprops enabled and disable everything else explicitly.
>>
>> Why not use a bitfield (with AV_OPT_TYPE_FLAGS and AV_OPT_TYPE_CONST)?
>> These properties certainly seem like a bitfield and the above would be
>> easily achievable with it.
>>
>
> How are flags supposed to be set to on by default? With av_dict_set or
> a different mechanism? Because the closest I was able to get¹,
>
> if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER |
> AVISYNTH_FRAMEPROP_DEFAULT) {
>
> ends up correctly setting the flags that should be on by default,
> with the new sar flag able to be enabled or disabled, but the ones
> flagged as default refuse to be disabled now.
>
> ¹https://github.com/qyot27/FFmpeg/commit/6d3d6108145f9c7ac2dfcdaf09852b7472f6ca7f
>
{ "avisynth_flags", "set flags related to reading frame properties from
script (AviSynth+ v3.7.1 or higher)", OFFSET(flags), AV_OPT_TYPE_FLAGS,
{.i64 = 1}, 0, 1, AV_OPT_FLAG_DECODING_PARAM, "flags" },
This is wrong. It should be
{ "avisynth_flags", "set flags related to reading frame properties from
script (AviSynth+ v3.7.1 or higher)", OFFSET(flags), AV_OPT_TYPE_FLAGS,
{.i64 = AVISYNTH_FRAMEPROP_FIELD_ORDER | AVISYNTH_FRAMEPROP_RANGE |
AVISYNTH_FRAMEPROP_PRIMARIES | AVISYNTH_FRAMEPROP_TRANSFER |
AVISYNTH_FRAMEPROP_MATRIX | AVISYNTH_FRAMEPROP_CHROMA_LOCATION}, 0,
INT_MAX, AV_OPT_FLAG_DECODING_PARAM, "flags" }
The default option should be removed. Users can then set the options via
avisynth_flags=+sar-range or via avisynth_flags=matrix or however they wish.
The AVISYNTH_FRAMEPROP_DEFAULT should also be removed (at least, it
should not be part of the bitfield, but you can of course add a define
(or an enum value) equivalent to the default value I used above and you
can use that for the default value above); to know whether field order
should be exported, you simply query via "if (avs->flags &
AVISYNTH_FRAMEPROP_FIELD_ORDER)". For this it is of course important
that the default value is a combination of other bits of the bitfield
and not a bit of its own.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/9] avformat/avisynth: add read_frameprops option
On 8/28/22 8:41 PM, Andreas Rheinhardt wrote:
This will make frameprops a global on-off which overrides everything
else even if some of the "else" stuff has been enabled explicitly. Worse
yet, if you want to disable everything except exactly one field, you
have to leave frameprops enabled and disable everything else explicitly.
Why not use a bitfield (with AV_OPT_TYPE_FLAGS and AV_OPT_TYPE_CONST)?
These properties certainly seem like a bitfield and the above would be
easily achievable with it.
How are flags supposed to be set to on by default? With av_dict_set or
a different mechanism? Because the closest I was able to get¹,
if(avs->flags & AVISYNTH_FRAMEPROP_FIELD_ORDER |
AVISYNTH_FRAMEPROP_DEFAULT) {
ends up correctly setting the flags that should be on by default,
with the new sar flag able to be enabled or disabled, but the ones
flagged as default refuse to be disabled now.
¹https://github.com/qyot27/FFmpeg/commit/6d3d6108145f9c7ac2dfcdaf09852b7472f6ca7f
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Leo Izen (12022-08-30): > Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = foo()? > Most other APIs return pointers to structs, rather than structs themselves > (see: av_packet_alloc). Using a pointer would prevent us from having > sizeof(AVWriter) as part of the ABI, as was done with AVPacket. Yes: to return a pointer, you need somewhere to store the structure. One of the point of AVWriter is that you can store it on the stack to avoid dynamic allocations when the string is short enough. Note that AVWriter is exactly two pointers. It will always be two pointers, and all the objects that I intend to introduce later will always be two pointers: one const pointer for the methods, one pointer for the object itself. This design is essential to the features I promised for AVWriter and for later. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On 8/24/22 11:18, Nicolas George wrote:
+```
+AVWriter wr = av_dynbuf_writer();
+av_something_write(wr, something, 0);
+if (av_writer_get_error(wr, 0) < 0)
+die("Failed");
+use_string(av_dynbuf_writer_get_data(wr, NULL));
+av_dynbuf_writer_finalize(wr, NULL, NULL);
+```
Is there a reason this is AVWriter wr = foo() and not AVWriter *wr =
foo()? Most other APIs return pointers to structs, rather than structs
themselves (see: av_packet_alloc). Using a pointer would prevent us from
having sizeof(AVWriter) as part of the ABI, as was done with AVPacket.
- Leo Izen (thebombzen)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] fftools/cmdutils: Add function to report error before exit
Andreas Rheinhardt:
> This is designed to improve and unify error handling for
> allocation failures for the many (often small) allocations that we have
> in the fftools. These typically either don't return an error message
> or an error message that is not really helpful to the user
> and can be replaced by a generic error message without loss of
> information.
>
> Reviewed-by: James Almer
> Signed-off-by: Andreas Rheinhardt
> ---
> fftools/cmdutils.c | 6 ++
> fftools/cmdutils.h | 11 +++
> 2 files changed, 17 insertions(+)
>
> diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
> index 18e768b386..da3d391694 100644
> --- a/fftools/cmdutils.c
> +++ b/fftools/cmdutils.c
> @@ -90,6 +90,12 @@ void register_exit(void (*cb)(int ret))
> program_exit = cb;
> }
>
> +void report_and_exit(int ret)
> +{
> +av_log(NULL, AV_LOG_FATAL, "%s\n", av_err2str(ret));
> +exit_program(AVUNERROR(ret));
> +}
> +
> void exit_program(int ret)
> {
> if (program_exit)
> diff --git a/fftools/cmdutils.h b/fftools/cmdutils.h
> index d87e162ccd..4496221983 100644
> --- a/fftools/cmdutils.h
> +++ b/fftools/cmdutils.h
> @@ -54,6 +54,17 @@ extern int hide_banner;
> */
> void register_exit(void (*cb)(int ret));
>
> +/**
> + * Reports an error corresponding to the provided
> + * AVERROR code and calls exit_program() with the
> + * corresponding POSIX error code.
> + * @note ret must be an AVERROR-value of a POSIX error code
> + * (i.e. AVERROR(EFOO) and not AVERROR_FOO).
> + * library functions can return both, so call this only
> + * with AVERROR(EFOO) of your own.
> + */
> +void report_and_exit(int ret) av_noreturn;
> +
> /**
> * Wraps exit with a program-specific cleanup routine.
> */
Will apply tonight unless there are objections.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Nicolas George (12022-08-24): > The actual implementation, tests and uses in the rest of > FFmpeg code will be committed separately once the API is > settled. > > Signed-off-by: Nicolas George > --- > doc/avwriter_intro.md | 109 ++ > libavutil/writer.h| 484 ++ > 2 files changed, 593 insertions(+) > create mode 100644 doc/avwriter_intro.md > create mode 100644 libavutil/writer.h > > > As suggested by JB, here is the header and documentation for AVWriter, > to discuss the principle before investing time in polishing the > implementation. I expect to discuss the API and if no blockign > objections are raised push it then spend time on the implementation. > > This API is a nicer and much more powerful version of BPrint. It can be > used to simplify existing code where BPrint could help, plus places > where BPrint could not help. > > It also is the prerequisite for more ambitious projects, expecially > universal serialization of FFmpeg objects (side data and such) into > standardized formats. > > The implementation is in most part done, and I am sure I can deliver. > What remains is the boring part of integrating the tests in FATE, > polishing various parts, updating the parts where I changed my mind > midway, etc. > > Note: FF_NEW_SZ is a macro defined elsewhere; it is really part of the > implementation more than the API. > > Ping. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec: add a bsf to reorder DTS into PTS
James Almer:
> On 8/30/2022 11:30 AM, Andreas Rheinhardt wrote:
>> James Almer:
>>> Starting with an h264 implementation. Can be extended to support
>>> other codecs.
>>>
>>> Addresses ticket #502.
>>>
>>> Signed-off-by: James Almer
>>> ---
>>> configure | 1 +
>>> libavcodec/Makefile | 1 +
>>> libavcodec/bitstream_filters.c | 1 +
>>> libavcodec/dts2pts_bsf.c | 477 +
>>> 4 files changed, 480 insertions(+)
>>> create mode 100644 libavcodec/dts2pts_bsf.c
>>>
>>> diff --git a/configure b/configure
>>> index 932ea5b553..91ee5eb303 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -3275,6 +3275,7 @@ aac_adtstoasc_bsf_select="adts_header mpeg4audio"
>>> av1_frame_merge_bsf_select="cbs_av1"
>>> av1_frame_split_bsf_select="cbs_av1"
>>> av1_metadata_bsf_select="cbs_av1"
>>> +dts2pts_bsf_select="cbs_h264 h264parse"
>>> eac3_core_bsf_select="ac3_parser"
>>> filter_units_bsf_select="cbs"
>>> h264_metadata_bsf_deps="const_nan"
>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>> index cb80f73d99..858e110b79 100644
>>> --- a/libavcodec/Makefile
>>> +++ b/libavcodec/Makefile
>>> @@ -1176,6 +1176,7 @@ OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF) +=
>>> av1_frame_split_bsf.o
>>> OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o
>>> OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o
>>> OBJS-$(CONFIG_DCA_CORE_BSF) += dca_core_bsf.o
>>> +OBJS-$(CONFIG_DTS2PTS_BSF) += dts2pts_bsf.o
>>> OBJS-$(CONFIG_DV_ERROR_MARKER_BSF) += dv_error_marker_bsf.o
>>> OBJS-$(CONFIG_EAC3_CORE_BSF) += eac3_core_bsf.o
>>> OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF) +=
>>> extract_extradata_bsf.o \
>>> diff --git a/libavcodec/bitstream_filters.c
>>> b/libavcodec/bitstream_filters.c
>>> index 23ae93..a3bebefe5f 100644
>>> --- a/libavcodec/bitstream_filters.c
>>> +++ b/libavcodec/bitstream_filters.c
>>> @@ -31,6 +31,7 @@ extern const FFBitStreamFilter ff_av1_metadata_bsf;
>>> extern const FFBitStreamFilter ff_chomp_bsf;
>>> extern const FFBitStreamFilter ff_dump_extradata_bsf;
>>> extern const FFBitStreamFilter ff_dca_core_bsf;
>>> +extern const FFBitStreamFilter ff_dts2pts_bsf;
>>> extern const FFBitStreamFilter ff_dv_error_marker_bsf;
>>> extern const FFBitStreamFilter ff_eac3_core_bsf;
>>> extern const FFBitStreamFilter ff_extract_extradata_bsf;
>>> diff --git a/libavcodec/dts2pts_bsf.c b/libavcodec/dts2pts_bsf.c
>>> new file mode 100644
>>> index 00..f600150a6b
>>> --- /dev/null
>>> +++ b/libavcodec/dts2pts_bsf.c
>>> @@ -0,0 +1,477 @@
>>> +/*
>>> + * Copyright (c) 2022 James Almer
>>> + *
>>> + * 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
>>> + * Derive PTS by reordering DTS from supported streams
>>> + */
>>> +
>>> +#include "libavutil/avassert.h"
>>> +#include "libavutil/eval.h"
Unused.
>>> +#include "libavutil/fifo.h"
>>> +#include "libavutil/opt.h"
>>> +#include "libavutil/tree.h"
>>> +
>>> +#include "bsf.h"
>>> +#include "bsf_internal.h"
>>> +#include "cbs.h"
>>> +#include "cbs_h264.h"
>>> +#include "h264_parse.h"
>>> +#include "h264_ps.h"
>>> +
>>> +typedef struct DTS2PTSNode {
>>> + int64_t dts;
>>> + int64_t duration;
>>> + int poc;
>>> +} DTS2PTSNode;
>>> +
>>> +typedef struct DTS2PTSFrame {
>>> + AVPacket *pkt;
>>> + int poc;
>>> + int poc_diff;
>>> +} DTS2PTSFrame;
>>> +
>>> +typedef struct DTS2PTSH264Context {
>>> + H264POCContext poc;
>>> + SPS sps;
>>> + int last_poc;
>>> + int highest_poc;
>>> + int picture_structure;
>>> +} DTS2PTSH264Context;
>>> +
>>> +typedef struct DTS2PTSContext {
>>> + struct AVTreeNode *root;
>>> + AVFifo *fifo;
>>> +
>>> + // Codec specific function pointers
>>> + int (*init)(AVBSFContext *ctx);
>>> + int (*filter)(AVBSFContext *ctx);
>>> + void (*flush)(AVBSFContext *ctx);
>>> +
>>> + CodedBitstreamContext *cbc;
>>> + CodedBitstreamFragment au;
>>> +
>>> + union {
>>> + DTS2PTSH264Context h264;
>>> + } u;
>>> +
>>> + int nb_frame;
>>> + int eof;
>>> +} DTS2PTSContext;
>>> +
>>>
Re: [FFmpeg-devel] [PATCH] avcodec: add a bsf to reorder DTS into PTS
On 8/30/2022 11:30 AM, Andreas Rheinhardt wrote:
James Almer:
Starting with an h264 implementation. Can be extended to support other codecs.
Addresses ticket #502.
Signed-off-by: James Almer
---
configure | 1 +
libavcodec/Makefile| 1 +
libavcodec/bitstream_filters.c | 1 +
libavcodec/dts2pts_bsf.c | 477 +
4 files changed, 480 insertions(+)
create mode 100644 libavcodec/dts2pts_bsf.c
diff --git a/configure b/configure
index 932ea5b553..91ee5eb303 100755
--- a/configure
+++ b/configure
@@ -3275,6 +3275,7 @@ aac_adtstoasc_bsf_select="adts_header mpeg4audio"
av1_frame_merge_bsf_select="cbs_av1"
av1_frame_split_bsf_select="cbs_av1"
av1_metadata_bsf_select="cbs_av1"
+dts2pts_bsf_select="cbs_h264 h264parse"
eac3_core_bsf_select="ac3_parser"
filter_units_bsf_select="cbs"
h264_metadata_bsf_deps="const_nan"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index cb80f73d99..858e110b79 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1176,6 +1176,7 @@ OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF)+=
av1_frame_split_bsf.o
OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o
OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o
OBJS-$(CONFIG_DCA_CORE_BSF) += dca_core_bsf.o
+OBJS-$(CONFIG_DTS2PTS_BSF)+= dts2pts_bsf.o
OBJS-$(CONFIG_DV_ERROR_MARKER_BSF)+= dv_error_marker_bsf.o
OBJS-$(CONFIG_EAC3_CORE_BSF) += eac3_core_bsf.o
OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF) += extract_extradata_bsf.o\
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 23ae93..a3bebefe5f 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -31,6 +31,7 @@ extern const FFBitStreamFilter ff_av1_metadata_bsf;
extern const FFBitStreamFilter ff_chomp_bsf;
extern const FFBitStreamFilter ff_dump_extradata_bsf;
extern const FFBitStreamFilter ff_dca_core_bsf;
+extern const FFBitStreamFilter ff_dts2pts_bsf;
extern const FFBitStreamFilter ff_dv_error_marker_bsf;
extern const FFBitStreamFilter ff_eac3_core_bsf;
extern const FFBitStreamFilter ff_extract_extradata_bsf;
diff --git a/libavcodec/dts2pts_bsf.c b/libavcodec/dts2pts_bsf.c
new file mode 100644
index 00..f600150a6b
--- /dev/null
+++ b/libavcodec/dts2pts_bsf.c
@@ -0,0 +1,477 @@
+/*
+ * Copyright (c) 2022 James Almer
+ *
+ * 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
+ * Derive PTS by reordering DTS from supported streams
+ */
+
+#include "libavutil/avassert.h"
+#include "libavutil/eval.h"
+#include "libavutil/fifo.h"
+#include "libavutil/opt.h"
+#include "libavutil/tree.h"
+
+#include "bsf.h"
+#include "bsf_internal.h"
+#include "cbs.h"
+#include "cbs_h264.h"
+#include "h264_parse.h"
+#include "h264_ps.h"
+
+typedef struct DTS2PTSNode {
+int64_t dts;
+int64_t duration;
+int poc;
+} DTS2PTSNode;
+
+typedef struct DTS2PTSFrame {
+AVPacket*pkt;
+int poc;
+int poc_diff;
+} DTS2PTSFrame;
+
+typedef struct DTS2PTSH264Context {
+H264POCContext poc;
+SPS sps;
+int last_poc;
+int highest_poc;
+int picture_structure;
+} DTS2PTSH264Context;
+
+typedef struct DTS2PTSContext {
+struct AVTreeNode *root;
+AVFifo *fifo;
+
+// Codec specific function pointers
+int (*init)(AVBSFContext *ctx);
+int (*filter)(AVBSFContext *ctx);
+void (*flush)(AVBSFContext *ctx);
+
+CodedBitstreamContext *cbc;
+CodedBitstreamFragment au;
+
+union {
+DTS2PTSH264Context h264;
+} u;
+
+int nb_frame;
+int eof;
+} DTS2PTSContext;
+
+// AVTreeNode callbacks
+static int cmp_insert(const void *key, const void *node)
+{
+return ((const DTS2PTSNode *) key)->poc - ((const DTS2PTSNode *)
node)->poc;
+}
+
+static int cmp_find(const void *key, const void *node)
+{
+return *(const int *)key - ((const DTS2PTSNode *) node)->poc;
+}
+
+static int dec_poc(void *opaque, void *elem)
+{
+DTS2PTSNode *node = elem;
+int dec = *(int *)opaque;
+node->poc -= dec;
+return 0;
+}
+
+static int free_node(void *opaque, void *elem)
+{
+DTS2PTSNode *node = elem;
+av_free(node);
+return 0;
Re: [FFmpeg-devel] [PATCH] avcodec: add a bsf to reorder DTS into PTS
James Almer:
> Starting with an h264 implementation. Can be extended to support other codecs.
>
> Addresses ticket #502.
>
> Signed-off-by: James Almer
> ---
> configure | 1 +
> libavcodec/Makefile| 1 +
> libavcodec/bitstream_filters.c | 1 +
> libavcodec/dts2pts_bsf.c | 477 +
> 4 files changed, 480 insertions(+)
> create mode 100644 libavcodec/dts2pts_bsf.c
>
> diff --git a/configure b/configure
> index 932ea5b553..91ee5eb303 100755
> --- a/configure
> +++ b/configure
> @@ -3275,6 +3275,7 @@ aac_adtstoasc_bsf_select="adts_header mpeg4audio"
> av1_frame_merge_bsf_select="cbs_av1"
> av1_frame_split_bsf_select="cbs_av1"
> av1_metadata_bsf_select="cbs_av1"
> +dts2pts_bsf_select="cbs_h264 h264parse"
> eac3_core_bsf_select="ac3_parser"
> filter_units_bsf_select="cbs"
> h264_metadata_bsf_deps="const_nan"
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index cb80f73d99..858e110b79 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1176,6 +1176,7 @@ OBJS-$(CONFIG_AV1_FRAME_SPLIT_BSF)+=
> av1_frame_split_bsf.o
> OBJS-$(CONFIG_CHOMP_BSF) += chomp_bsf.o
> OBJS-$(CONFIG_DUMP_EXTRADATA_BSF) += dump_extradata_bsf.o
> OBJS-$(CONFIG_DCA_CORE_BSF) += dca_core_bsf.o
> +OBJS-$(CONFIG_DTS2PTS_BSF)+= dts2pts_bsf.o
> OBJS-$(CONFIG_DV_ERROR_MARKER_BSF)+= dv_error_marker_bsf.o
> OBJS-$(CONFIG_EAC3_CORE_BSF) += eac3_core_bsf.o
> OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF) += extract_extradata_bsf.o\
> diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
> index 23ae93..a3bebefe5f 100644
> --- a/libavcodec/bitstream_filters.c
> +++ b/libavcodec/bitstream_filters.c
> @@ -31,6 +31,7 @@ extern const FFBitStreamFilter ff_av1_metadata_bsf;
> extern const FFBitStreamFilter ff_chomp_bsf;
> extern const FFBitStreamFilter ff_dump_extradata_bsf;
> extern const FFBitStreamFilter ff_dca_core_bsf;
> +extern const FFBitStreamFilter ff_dts2pts_bsf;
> extern const FFBitStreamFilter ff_dv_error_marker_bsf;
> extern const FFBitStreamFilter ff_eac3_core_bsf;
> extern const FFBitStreamFilter ff_extract_extradata_bsf;
> diff --git a/libavcodec/dts2pts_bsf.c b/libavcodec/dts2pts_bsf.c
> new file mode 100644
> index 00..f600150a6b
> --- /dev/null
> +++ b/libavcodec/dts2pts_bsf.c
> @@ -0,0 +1,477 @@
> +/*
> + * Copyright (c) 2022 James Almer
> + *
> + * 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
> + * Derive PTS by reordering DTS from supported streams
> + */
> +
> +#include "libavutil/avassert.h"
> +#include "libavutil/eval.h"
> +#include "libavutil/fifo.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/tree.h"
> +
> +#include "bsf.h"
> +#include "bsf_internal.h"
> +#include "cbs.h"
> +#include "cbs_h264.h"
> +#include "h264_parse.h"
> +#include "h264_ps.h"
> +
> +typedef struct DTS2PTSNode {
> +int64_t dts;
> +int64_t duration;
> +int poc;
> +} DTS2PTSNode;
> +
> +typedef struct DTS2PTSFrame {
> +AVPacket*pkt;
> +int poc;
> +int poc_diff;
> +} DTS2PTSFrame;
> +
> +typedef struct DTS2PTSH264Context {
> +H264POCContext poc;
> +SPS sps;
> +int last_poc;
> +int highest_poc;
> +int picture_structure;
> +} DTS2PTSH264Context;
> +
> +typedef struct DTS2PTSContext {
> +struct AVTreeNode *root;
> +AVFifo *fifo;
> +
> +// Codec specific function pointers
> +int (*init)(AVBSFContext *ctx);
> +int (*filter)(AVBSFContext *ctx);
> +void (*flush)(AVBSFContext *ctx);
> +
> +CodedBitstreamContext *cbc;
> +CodedBitstreamFragment au;
> +
> +union {
> +DTS2PTSH264Context h264;
> +} u;
> +
> +int nb_frame;
> +int eof;
> +} DTS2PTSContext;
> +
> +// AVTreeNode callbacks
> +static int cmp_insert(const void *key, const void *node)
> +{
> +return ((const DTS2PTSNode *) key)->poc - ((const DTS2PTSNode *)
> node)->poc;
> +}
> +
> +static int cmp_find(const void *key, const void *node)
> +{
> +return *(const int *)key - ((const DTS2PTSNode *) node)->poc;
> +}
> +
> +static int dec_poc(void *opaque, void *elem)
> +{
Re: [FFmpeg-devel] [PATCH 2/3] lavu/fifo: clarify interaction of AV_FIFO_FLAG_AUTO_GROW with av_fifo_can_write()
Quoting James Almer (2022-08-30 14:56:45) > > > > I disagree that this is a break. > > > > The issue in my view is that 'can be written' is ambiguous here, so we > > are interpreting it differently. Your interpretation is apparently > > 'maximum number of elements for which a write can possibly succeeed', > > whereas my intended interpretation was 'maximum number of elements for > > which a write is always guaranteed to succeed'. > > IMO it's not really ambiguous. If you don't state that's the intention, > which you're doing in this patch, then "can be written" has one literal > meaning. I would disagree here. Consider an autogrowing fifo in an out-of-memory situation. What "can be written" into it? > > One of these interpretations is correct, because it matches the actual > > behaviour. So the right solution IMO is to clarify the documentation so > > it is no longer ambiguous, but I do not consider this an API break. > > av_fifo_write() says "In case nb_elems > av_fifo_can_write(f), nothing > is written and an error is returned.", which is definitely not > ambiguous, and you're changing it in patch 3/3 to include the case where > having enabled autogrow could result in the function succeeding when > nb_elems > av_fifo_can_write(f). That is quite clearly a bug in the documentation IMO. That line was not present in the original patches I sent, but added at some time later in the development (don't remember whether by myself or Andreas); then whichever of us added it forgot to update it in the patch adding AV_FIFO_FLAG_AUTO_GROW. > The behavior of the function remains intact, but a library user reading > the documentation in ffmpeg 5.1 and the documentation in what will be > 5.2 after this patch could rightly assume the function was changed and > will behave differently between versions (Which is not the case, but to > find out you'll have to read the implementation, or the git history, or > test code with both versions). So this is technically an API break. Technically yes, but the unfortunate fact of the matter is that our API documentation simply is not, and never was, sufficiently complete and precise to be the sole source of truth. Plenty of things are missing, obsolete, inconsistent, and sometimes just wrong. I wish it were otherwise, and I believe the situation is slowly improving, but we just don't have the resources to make our docs anywhere close to perfect any time soon. So unfortunately people have to test their code, and testing in this case would immediately reveal how it actually works. As a consequence we have to be pragmatic when choosing whether to change code to match the docs or vice versa. > > > > > More generally: > > - a FIFO conceptually has a well-defined size at any given moment > > - that size is can_read() + can_write() > > But this could (should?) have been av_fifo_size2(). That way can_write() > could effectively become a generic "can write", instead of begin stuck > as "can write without the chance of failure". Maybe, but it's a bit late for that. Actually I remember considering an av_fifo_size2(), but then decided against it, probably because it could confuse people into thinking it's like av_fifo_size(), which it most definitely is not. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] lavu/fifo: clarify interaction of AV_FIFO_FLAG_AUTO_GROW with av_fifo_can_write()
On 8/30/2022 3:35 AM, Anton Khirnov wrote: Quoting James Almer (2022-08-29 17:00:54) On 8/29/2022 11:07 AM, Anton Khirnov wrote: --- libavutil/fifo.h | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libavutil/fifo.h b/libavutil/fifo.h index 6c6bd78842..89872d0972 100644 --- a/libavutil/fifo.h +++ b/libavutil/fifo.h @@ -97,7 +97,13 @@ void av_fifo_auto_grow_limit(AVFifo *f, size_t max_elems); size_t av_fifo_can_read(const AVFifo *f); /** - * @return number of elements that can be written into the given FIFO. + * @return Number of elements that can be written into the given FIFO without + * growing it. + * + * In other words, this number of elements or less is guaranteed to fit + * into the FIFO. More data may be written when the + * AV_FIFO_FLAG_AUTO_GROW flag was specified at FIFO creation, but this + * may involve memory allocation, which can fail. This patch is an API break, because before it i was told av_fifo_can_write() would tell me the amount of elements i could write into the FIFO, regardless of how it was created, but now it legitimates the one scenario where it was not reliable. An scenario i stumbled upon in my code by following the documentation, which is in at least one release, the LTS one. Instead of changing the documentation to fit the behavior, the behavior should match the documentation. This means that if a call to av_fifo_write() can succeed, then av_fifo_can_write() should reflect that. That said, it would be great if making av_fifo_can_write() tell the real amount of elements one can write into the FIFO was possible without breaking anything, but the doxy for av_fifo_grow2() says "On success, the FIFO will be large enough to hold exactly inc + av_fifo_can_read() + av_fifo_can_write()", a line that was obviously aware of the fact av_fifo_can_write() ignored the autogrow feature, and would no longer be true if said function is fixed. I disagree that this is a break. The issue in my view is that 'can be written' is ambiguous here, so we are interpreting it differently. Your interpretation is apparently 'maximum number of elements for which a write can possibly succeeed', whereas my intended interpretation was 'maximum number of elements for which a write is always guaranteed to succeed'. IMO it's not really ambiguous. If you don't state that's the intention, which you're doing in this patch, then "can be written" has one literal meaning. One of these interpretations is correct, because it matches the actual behaviour. So the right solution IMO is to clarify the documentation so it is no longer ambiguous, but I do not consider this an API break. av_fifo_write() says "In case nb_elems > av_fifo_can_write(f), nothing is written and an error is returned.", which is definitely not ambiguous, and you're changing it in patch 3/3 to include the case where having enabled autogrow could result in the function succeeding when nb_elems > av_fifo_can_write(f). The behavior of the function remains intact, but a library user reading the documentation in ffmpeg 5.1 and the documentation in what will be 5.2 after this patch could rightly assume the function was changed and will behave differently between versions (Which is not the case, but to find out you'll have to read the implementation, or the git history, or test code with both versions). So this is technically an API break. More generally: - a FIFO conceptually has a well-defined size at any given moment - that size is can_read() + can_write() But this could (should?) have been av_fifo_size2(). That way can_write() could effectively become a generic "can write", instead of begin stuck as "can write without the chance of failure". - you can change the size by growing the FIFO - AV_FIFO_FLAG_AUTO_GROW does not fundamentally change the above concepts, it merely calls av_fifo_grow2() when a write would otherwise fail Now we can introduce a function for 'maximum number that can possibly succeed' if you think it's useful - something like av_fifo_max_write(). Maybe, but only if we find a usecase for it. Hendrik had an opinion about what av_fifo_can_write() should report, but it was on IRC. But otherwise, knowing that anything we do will be breaking, if the current behavior was your intention all along as the author of the API, then i guess this set can go in unless someone else chimes. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/2] avformat/tests: Add a test for avio_check with the pipe protocol
Creates a UNIX pipe and then verifies that avio_check returns the right
access flags for the two ends of the pipe.
v2: Add support for the Windows version of _pipe
Signed-off-by: Neil Roberts
---
libavformat/Makefile | 1 +
libavformat/tests/.gitignore | 1 +
libavformat/tests/pipe.c | 108 +++
tests/fate/libavformat.mak | 5 ++
4 files changed, 115 insertions(+)
create mode 100644 libavformat/tests/pipe.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index f67a99f839..9c681c58c5 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -732,6 +732,7 @@ TESTPROGS-$(CONFIG_MOV_MUXER)+= movenc
TESTPROGS-$(CONFIG_NETWORK) += noproxy
TESTPROGS-$(CONFIG_SRTP) += srtp
TESTPROGS-$(CONFIG_IMF_DEMUXER) += imf
+TESTPROGS-$(CONFIG_PIPE_PROTOCOL)+= pipe
TOOLS = aviocat \
ismindex\
diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore
index cdd0cce061..567d6f9e40 100644
--- a/libavformat/tests/.gitignore
+++ b/libavformat/tests/.gitignore
@@ -7,3 +7,4 @@
/srtp
/url
/seek_utils
+/pipe
diff --git a/libavformat/tests/pipe.c b/libavformat/tests/pipe.c
new file mode 100644
index 00..18a8551fd5
--- /dev/null
+++ b/libavformat/tests/pipe.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2022 Neil Roberts
+ *
+ * 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
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "libavformat/avio.h"
+#include "libavutil/error.h"
+
+static int check_pipe(const char *url, int mask, int expected)
+{
+int flags = avio_check(url, mask);
+
+if (flags < 0) {
+fprintf(stderr,
+"avio_check for %s with mask 0x%x failed: %s\n",
+url,
+mask,
+av_err2str(flags));
+return 0;
+}
+
+if (flags != expected) {
+fprintf(stderr,
+"Wrong result returned from avio_check for %s with mask 0x%x. "
+"Expected 0x%x but received 0x%x\n",
+url,
+mask,
+expected,
+flags);
+return 0;
+}
+
+return 1;
+}
+
+int main(int argc, char **argv)
+{
+int ret = 0;
+int pipe_fds[2];
+char read_url[20], write_url[20];
+int check_invalid_ret;
+int pipe_ret;
+
+#ifdef _WIN32
+pipe_ret = _pipe(pipe_fds, 1024 /* psize */, 0 /* textmode */);
+#else
+pipe_ret = pipe(pipe_fds);
+#endif
+
+if (pipe_ret == -1) {
+fprintf(stderr, "error creating pipe: %s\n", strerror(errno));
+return 1;
+}
+
+snprintf(read_url, sizeof(read_url), "pipe:%d", pipe_fds[0]);
+snprintf(write_url, sizeof(write_url), "pipe:%d", pipe_fds[1]);
+
+if (!check_pipe(read_url,
+AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+AVIO_FLAG_READ))
+ret = 1;
+
+if (!check_pipe(write_url,
+AVIO_FLAG_READ | AVIO_FLAG_WRITE,
+AVIO_FLAG_WRITE))
+ret = 1;
+
+/* Ensure that we don't get flags that we didn't ask for */
+if (!check_pipe(read_url, AVIO_FLAG_WRITE, 0))
+ret = 1;
+
+close(pipe_fds[0]);
+close(pipe_fds[1]);
+
+/* An invalid fd should return EBADF */
+check_invalid_ret = avio_check(read_url, AVIO_FLAG_READ);
+
+if (check_invalid_ret != AVERROR(EBADF)) {
+fprintf(stderr,
+"avio_check on invalid FD expected to return %i "
+"but %i was received\n",
+AVERROR(EBADF),
+check_invalid_ret);
+ret = 1;
+}
+
+return ret;
+}
diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak
index d2acb4c9e0..7a22f54c04 100644
--- a/tests/fate/libavformat.mak
+++ b/tests/fate/libavformat.mak
@@ -26,6 +26,11 @@ FATE_LIBAVFORMAT-$(CONFIG_IMF_DEMUXER) += fate-imf
fate-imf: libavformat/tests/imf$(EXESUF)
fate-imf: CMD = run libavformat/tests/imf$(EXESUF)
+FATE_LIBAVFORMAT-$(CONFIG_PIPE_PROTOCOL) += fate-pipe
+fate-pipe: libavformat/tests/pipe$(EXESUF)
+fate-pipe: CMD = run libavformat/tests/pipe$(EXESUF)
[FFmpeg-devel] [PATCH v2 1/2] avformat/file: Add a specialized url_check callback for pipe protocol
Using file_check for the pipe protocol doesn't make sense. For example,
for a URL like “pipe:0” it would end up checking whether the “pipe:0”
file exists. This patch instead makes it check the access modes on the
corresponding file descriptor using fcntl on *nix and DuplicateHandle on
Windows.
v2: Use DuplicateHandle on Windows to check whether the duplicated
handle can have the corresponding access flags.
Signed-off-by: Neil Roberts
---
libavformat/file.c | 74 --
1 file changed, 71 insertions(+), 3 deletions(-)
diff --git a/libavformat/file.c b/libavformat/file.c
index 98c9e81bcb..b3f7bc9282 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -375,9 +375,8 @@ const URLProtocol ff_file_protocol = {
#if CONFIG_PIPE_PROTOCOL
-static int pipe_open(URLContext *h, const char *filename, int flags)
+static int pipe_get_fd(const char *filename, int flags)
{
-FileContext *c = h->priv_data;
int fd;
char *final;
av_strstart(filename, "pipe:", );
@@ -390,6 +389,14 @@ static int pipe_open(URLContext *h, const char *filename,
int flags)
fd = 0;
}
}
+
+return fd;
+}
+
+static int pipe_open(URLContext *h, const char *filename, int flags)
+{
+FileContext *c = h->priv_data;
+int fd = pipe_get_fd(filename, flags);
#if HAVE_SETMODE
setmode(fd, O_BINARY);
#endif
@@ -398,13 +405,74 @@ static int pipe_open(URLContext *h, const char *filename,
int flags)
return 0;
}
+static int pipe_check(URLContext *h, int mask)
+{
+int fd = pipe_get_fd(h->filename, mask);
+int access = 0;
+
+#ifdef _WIN32
+
+HANDLE pipe_handle = (HANDLE) _get_osfhandle(fd);
+HANDLE tmp_handle;
+
+if (pipe_handle == INVALID_HANDLE_VALUE)
+return AVERROR(errno);
+
+if (mask & AVIO_FLAG_READ &&
+DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
+pipe_handle,
+GetCurrentProcess(), /* hTargetProcessHandle */
+_handle,
+FILE_READ_DATA,
+FALSE, /* bInheritHandle */
+0 /* options */)) {
+access |= AVIO_FLAG_READ;
+CloseHandle(tmp_handle);
+}
+
+if (mask & AVIO_FLAG_WRITE &&
+DuplicateHandle(GetCurrentProcess(), /* hSourceProcessHandle */
+pipe_handle,
+GetCurrentProcess(), /* hTargetProcessHandle */
+_handle,
+FILE_WRITE_DATA,
+FALSE, /* bInheritHandle */
+0 /* options */)) {
+access |= AVIO_FLAG_WRITE;
+CloseHandle(tmp_handle);
+}
+
+#else /* _WIN32 */
+
+int status_flags = fcntl(fd, F_GETFL);
+
+if (status_flags == -1)
+return AVERROR(errno);
+
+switch (status_flags & O_ACCMODE) {
+case O_RDONLY:
+access = AVIO_FLAG_READ;
+break;
+case O_WRONLY:
+access = AVIO_FLAG_WRITE;
+break;
+case O_RDWR:
+access = AVIO_FLAG_READ | AVIO_FLAG_WRITE;
+break;
+}
+
+#endif /* _WIN32 */
+
+return access & mask;
+}
+
const URLProtocol ff_pipe_protocol = {
.name= "pipe",
.url_open= pipe_open,
.url_read= file_read,
.url_write = file_write,
.url_get_file_handle = file_get_handle,
-.url_check = file_check,
+.url_check = pipe_check,
.priv_data_size = sizeof(FileContext),
.priv_data_class = _class,
.default_whitelist = "crypto,data"
--
2.37.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] lavu/fifo: clarify interaction of AV_FIFO_FLAG_AUTO_GROW with av_fifo_can_write()
Quoting James Almer (2022-08-29 17:00:54) > On 8/29/2022 11:07 AM, Anton Khirnov wrote: > > --- > > libavutil/fifo.h | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/libavutil/fifo.h b/libavutil/fifo.h > > index 6c6bd78842..89872d0972 100644 > > --- a/libavutil/fifo.h > > +++ b/libavutil/fifo.h > > @@ -97,7 +97,13 @@ void av_fifo_auto_grow_limit(AVFifo *f, size_t > > max_elems); > > size_t av_fifo_can_read(const AVFifo *f); > > > > /** > > - * @return number of elements that can be written into the given FIFO. > > + * @return Number of elements that can be written into the given FIFO > > without > > + * growing it. > > + * > > + * In other words, this number of elements or less is guaranteed > > to fit > > + * into the FIFO. More data may be written when the > > + * AV_FIFO_FLAG_AUTO_GROW flag was specified at FIFO creation, but > > this > > + * may involve memory allocation, which can fail. > > This patch is an API break, because before it i was told > av_fifo_can_write() would tell me the amount of elements i could write > into the FIFO, regardless of how it was created, but now it legitimates > the one scenario where it was not reliable. An scenario i stumbled upon > in my code by following the documentation, which is in at least one > release, the LTS one. > > Instead of changing the documentation to fit the behavior, the behavior > should match the documentation. This means that if a call to > av_fifo_write() can succeed, then av_fifo_can_write() should reflect that. > > That said, it would be great if making av_fifo_can_write() tell the real > amount of elements one can write into the FIFO was possible without > breaking anything, but the doxy for av_fifo_grow2() says "On success, > the FIFO will be large enough to hold exactly inc + av_fifo_can_read() + > av_fifo_can_write()", a line that was obviously aware of the fact > av_fifo_can_write() ignored the autogrow feature, and would no longer be > true if said function is fixed. I disagree that this is a break. The issue in my view is that 'can be written' is ambiguous here, so we are interpreting it differently. Your interpretation is apparently 'maximum number of elements for which a write can possibly succeeed', whereas my intended interpretation was 'maximum number of elements for which a write is always guaranteed to succeed'. One of these interpretations is correct, because it matches the actual behaviour. So the right solution IMO is to clarify the documentation so it is no longer ambiguous, but I do not consider this an API break. More generally: - a FIFO conceptually has a well-defined size at any given moment - that size is can_read() + can_write() - you can change the size by growing the FIFO - AV_FIFO_FLAG_AUTO_GROW does not fundamentally change the above concepts, it merely calls av_fifo_grow2() when a write would otherwise fail Now we can introduce a function for 'maximum number that can possibly succeed' if you think it's useful - something like av_fifo_max_write(). -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
