Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxf: rename sub_descriptors as file_descriptors
tis 2021-08-24 klockan 13:18 +0200 skrev Marc-Antoine Arnaud: --- libavformat/mxfdec.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) Fine by me /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
tor 2021-08-12 klockan 14:38 +0200 skrev Marc-Antoine Arnaud:
@@ -177,6 +179,7 @@ typedef struct {
int body_sid;
MXFWrappingScheme wrapping;
int edit_units_per_packet; /* how many edit units to read at a
time (PCM, ClipWrapped) */
+ int* channel_ordering;
Is there a maximum number of channels? If so then this should be made
constant size.
} MXFTrack;
typedef struct MXFDescriptor {
@@ -217,6 +220,15 @@ typedef struct MXFDescriptor {
size_t coll_size;
} MXFDescriptor;
+typedef struct MXFMCASubDescriptor {
+ MXFMetadataSet meta;
+ UID uid;
+ UID mca_link_id;
+ UID mca_group_link_id;
+ UID mca_label_dictionnary_id;
+ char *language;
Language doesn't seem to be used
+} MXFMCASubDescriptor;
+static inline int mxf_read_us_ascii_string(AVIOContext *pb, int size,
char** str)
+{
+ int ret;
+ size_t buf_size;
+
+ if (size < 0)
+ return AVERROR(EINVAL);
+
+ buf_size = size + 1;
+ av_free(*str);
+ *str = av_malloc(buf_size);
av_realloc()
static int mxf_parse_structural_metadata(MXFContext *mxf)
{
MXFPackage *material_package = NULL;
@@ -2322,7 +2461,10 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
const MXFCodecUL *pix_fmt_ul = NULL;
AVStream *st;
AVTimecode tc;
+ enum AVAudioServiceType *ast;
+ int* channel_ordering;
int flags;
+ int current_channel;
if (!(material_track = mxf_resolve_strong_ref(mxf,
_package->tracks_refs[i], Track))) {
av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material
track strong ref\n");
@@ -2681,6 +2823,185 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
st->internal->need_parsing = AVSTREAM_PARSE_FULL;
}
st->codecpar->bits_per_coded_sample =
av_get_bits_per_sample(st->codecpar->codec_id);
+
+ current_channel = 0;
+
+ channel_ordering = av_mallocz_array(descriptor->channels,
sizeof(int));
+
+ for (j = 0; j < descriptor->sub_descriptors_count; j++) {
+ MXFMCASubDescriptor *mca_sub_descriptor =
mxf_resolve_strong_ref(mxf, >sub_descriptors_refs[j],
MCASubDescriptor);
+ if (mca_sub_descriptor == NULL) {
+ continue;
+ }
+
+ // Soundfield group
+ if (IS_KLV_KEY(mca_sub_descriptor-
>mca_label_dictionnary_id, mxf_soundfield_group)) {
+ if (IS_KLV_KEY(mca_sub_descriptor-
>mca_label_dictionnary_id, mxf_soundfield_group_51)) {
+ st->codecpar->channel_layout =
AV_CH_LAYOUT_5POINT1;
+ continue;
+ }
Consider using a table lookup for this instead.
+
+ // Audio channel
+ if (IS_KLV_KEY(mca_sub_descriptor-
>mca_label_dictionnary_id, mxf_audio_channel)) {
+ if (IS_KLV_KEY(mca_sub_descriptor-
>mca_label_dictionnary_id, mxf_left_audio_channel)) {
+ channel_ordering[current_channel] = 0;
+ current_channel += 1;
+ }
Same here, except for the special case for the impaired stuff.
+
+ // TODO support other channels
+ // mxf_smpte_st2067_8_mono_one_audio_channel
+ // mxf_smpte_st2067_8_mono_two_audio_channel
+ // mxf_smpte_st2067_8_left_total_audio_channel
+ // mxf_smpte_st2067_8_right_total_audio_channel
+ //
mxf_smpte_st2067_8_left_surround_total_audio_channel
+ //
mxf_smpte_st2067_8_right_surround_total_audio_channel
+ // mxf_smpte_st2067_8_surround_audio_channel
These should be a ticket, not just a comment in the code. Or just
implement them :)
+ }
+
+ // set language from MCA spoken language information
+ // av_dict_set(>metadata, "language",
mca_sub_descriptor->language, 0);
Stray dead code. Is language not accurate perhaps?
+ }
+
+ // check if the mapping is not required
+ bool require_reordering = false;
+ for (j = 0; j < descriptor->channels; ++j) {
+ if (channel_ordering[j] != j) {
+ require_reordering = true;
+ break;
+ }
+ }
+
+ if (require_reordering && is_pcm(st->codecpar->codec_id))
{
+ current_channel = 0;
+ av_log(mxf->fc, AV_LOG_INFO, "MCA Audio mapping (");
+ for(j = 0; j < descriptor->channels; ++j) {
+ for(int k = 0; k < descriptor->channels; ++k) {
+ if(channel_ordering[k] == current_channel) {
+ av_log(mxf->fc, AV_LOG_INFO, "%d -> %d",
channel_ordering[k], k);
+ if (current_channel != descriptor-
>channels - 1)
+ av_log(mxf->fc,
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxf: support MCA audio information
@@ -2681,6 +2845,88 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
st->internal->need_parsing = AVSTREAM_PARSE_FULL;
}
st->codecpar->bits_per_coded_sample =
av_get_bits_per_sample(st->codecpar->codec_id);
+
+ current_channel = 0;
+
+
+
+ for (j = 0; j < FFMIN(descriptor->channels,
FF_SANE_NB_CHANNELS); ++j) {
I feel like we should error out with an appropriate message rather than
silently ignoring any extra channels. Otherwise this looks pretty good
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] avformat/mxfdec: check channel number in mxf_get_d10_aes3_packet()
sön 2021-09-05 klockan 21:24 +0200 skrev Michael Niedermayer: > Fixes: Out of array access > Fixes: 37030/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer- > 5387719147651072 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/mxfdec.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index 55f2e5c767..ebe411b04d 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -552,6 +552,10 @@ static int mxf_get_d10_aes3_packet(AVIOContext > *pb, AVStream *st, AVPacket *pkt, > data_ptr = pkt->data; > end_ptr = pkt->data + length; > buf_ptr = pkt->data + 4; /* skip SMPTE 331M header */ > + > + if (st->codecpar->channels > 8) > + return AVERROR_INVALIDDATA; Looks fine. Double-checked S331m, AES is limited to 8 channels /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 08/13] avcodec/elbg: Keep buffers to avoid allocations and frees
fre 2021-09-17 klockan 04:08 +0200 skrev Andreas Rheinhardt: > Up until now, each call to avpriv_elbg_do() would result > in at least six allocations. And this function is called a lot: > A typical FATE run results in 52213653 calls to av_malloc; of these, > 34974671 originate from av_malloc_array and from these 34783679 > originate from avpriv_elbg_do; the msvideo1 encoder tests are behind > most of these. > > This commit changes this by keeping the buffers and only reallocating > them when needed. E.g. for the encoding part of fate-vsynth1-msvideo1 > total heap usage went down from 11,407,939 allocs and frees with > 468,106,207 bytes allocated to 3,149 allocs and frees with 13,181,847 > bytes allocated. The time for one encode2-call went down by 69%. > > Signed-off-by: Andreas Rheinhardt Looks fine /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 10/13] avcodec/cinepakenc: Check all calls to avpriv_elbg_do()
fre 2021-09-17 klockan 04:08 +0200 skrev Andreas Rheinhardt: > Signed-off-by: Andreas Rheinhardt > --- > libavcodec/cinepakenc.c | 28 ++-- > 1 file changed, 22 insertions(+), 6 deletions(-) Gave this one a try with -vframes 100 -s 640x360 for some random clips on my machine. Wall time doesn't change much. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/13] avcodec/elbg: Remove avoidable buffer
fre 2021-09-17 klockan 04:02 +0200 skrev Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/elbg.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/elbg.c b/libavcodec/elbg.c
> index d012d9a384..795fc83f16 100644
> --- a/libavcodec/elbg.c
> +++ b/libavcodec/elbg.c
> @@ -376,7 +376,6 @@ int avpriv_do_elbg(int *points, int dim, int
> numpoints, int *codebook,
> elbg_data elbg_d;
> elbg_data *elbg = _d;
> int i, j, k, steps = 0, ret = 0;
> - int *dist_cb = av_malloc_array(numpoints, sizeof(int));
> int *size_part = av_malloc_array(numCB, sizeof(int));
> cell *list_buffer = av_malloc_array(numpoints, sizeof(cell));
> cell *free_cells;
> @@ -394,7 +393,7 @@ int avpriv_do_elbg(int *points, int dim, int
> numpoints, int *codebook,
> elbg->utility_inc = av_malloc_array(numCB, sizeof(*elbg-
> >utility_inc));
> elbg->scratchbuf = av_malloc_array(5*dim, sizeof(int));
>
> - if (!dist_cb || !size_part || !list_buffer || !elbg->cells ||
> + if (!size_part || !list_buffer || !elbg->cells ||
> !elbg->utility || !elbg->utility_inc || !elbg->scratchbuf) {
> ret = AVERROR(ENOMEM);
> goto out;
> @@ -423,9 +422,8 @@ int avpriv_do_elbg(int *points, int dim, int
> numpoints, int *codebook,
> }
> }
> elbg->nearest_cb[i] = best_idx;
> - dist_cb[i] = best_dist;
> - elbg->error += dist_cb[i];
> - elbg->utility[elbg->nearest_cb[i]] += dist_cb[i];
> + elbg->error += best_dist;
> + elbg->utility[elbg->nearest_cb[i]] += best_dist;
> free_cells->index = i;
> free_cells->next = elbg->cells[elbg->nearest_cb[i]];
> elbg->cells[elbg->nearest_cb[i]] = free_cells;
> @@ -453,7 +451,6 @@ int avpriv_do_elbg(int *points, int dim, int
> numpoints, int *codebook,
> (steps < max_steps));
>
> out:
> - av_free(dist_cb);
> av_free(size_part);
> av_free(elbg->utility);
> av_free(list_buffer);
Nice catch. Looks good to me
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 09/13] avcodec/elbg: Also allocate buffers for recursion only once
fre 2021-09-17 klockan 04:08 +0200 skrev Andreas Rheinhardt: > This is possible because the number of elements needed in each > recursion step decreases geometrically, so the geometric series > provides an upper bound for the sum of number of elements of > the needed buffers. Looks like a thing that could be verified formally via Frama-C. I might do this at some point. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxf: support MCA audio information
ons 2021-09-15 klockan 12:14 +0200 skrev Marc-Antoine Arnaud:
+static int mxf_read_mca_sub_descriptor(void *arg, AVIOContext *pb, int
tag, int size, UID uid, int64_t klv_offset)
+{
+ MXFMCASubDescriptor *mca_sub_descriptor = arg;
+
+ if (IS_KLV_KEY(uid, mxf_mca_prefix)) {
+ if (IS_KLV_KEY(uid, mxf_mca_label_dictionnary_id)) {
+ avio_read(pb, mca_sub_descriptor-
>mca_label_dictionnary_id, 16);
+ }
+ if (IS_KLV_KEY(uid, mxf_mca_link_id)) {
+ avio_read(pb, mca_sub_descriptor->mca_link_id, 16);
+ }
+ if (IS_KLV_KEY(uid, mxf_soundfield_group_link_id)) {
+ avio_read(pb, mca_sub_descriptor->mca_group_link_id, 16);
+ }
+ }
This nesting of ifs is unnecessary
Rest of the patch looks fine. Passes FATE. Do you have a sample or two?
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: make MXFMetadataSet part of all metadata sets
sön 2021-08-01 klockan 03:55 +0200 skrev Marton Balint:
> The code expects every kind of metadata set to start with the generic metadata
> set attributes.
>
> Signed-off-by: Marton Balint
> ---
> libavformat/mxfdec.c | 66 ++--
> 1 file changed, 21 insertions(+), 45 deletions(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index fd22680adb..34cbd2cd77 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -106,17 +106,19 @@ typedef struct MXFPartition {
> KLVPacket first_essence_klv;
> } MXFPartition;
>
> -typedef struct MXFCryptoContext {
> +typedef struct MXFMetadataSet {
> UID uid;
> MXFPartition *partition;
> enum MXFMetadataSetType type;
> +} MXFMetadataSet;
> +
> +typedef struct MXFCryptoContext {
> +MXFMetadataSet meta;
> UID source_container_ul;
> } MXFCryptoContext;
>
This is something I've had in mind as well, but never got around to. I
approve! This also makes the code more strict-aliasing correct
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Fwd: Re: [PATCH] mxfdec.c: prefer metadata from Footer
tor 2021-07-29 klockan 22:17 +0200 skrev Marton Balint: > On Fri, 23 Jul 2021, emco...@ffastrans.com wrote: > > > Am 2021-07-04 20:31, schrieb emco...@ffastrans.com: > > > Am 2021-07-04 20:28, schrieb emco...@ffastrans.com: > > > > Am 2021-07-04 17:28, schrieb Marton Balint: > > > > > On Sat, 3 Jul 2021, Tomas Härdin wrote: > > > > > > > > > > > lör 2021-07-03 klockan 15:13 +0200 skrev emco...@ffastrans.com: > > > > > > > Unfortunately the wetransfer link for the fate samples expired, > > > > > > > so > > > > > > > i thought it might be a good idea to resend it as link to gdrive: > > > > > > > > > https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing > > > > > > > Also attached the 2 patches: 1 from cus for mxfdec.c and one from > > > > > > > myself for the corresponding fate samples. > > > > > > > After applying the mxfdec.c patch, fate will pass with the > > > > > > > currently existing tests but the files in the zip must be > > > > > > > uploaded > > > > > > > to the fate suite before applying my corresponding patch of > > > > > > > course > > > > > > > (otherwise the files don't exist). > > > > > > > > > > > > > > It would be cool if someone found the time and wants to apply > > > > > > > this. > > > > > > > > > > > > The patch works (except for the samples not being in FATE) > > > > > > > > > > Actually I found one file where the packetization behaviour changes, > > > > > because after the patch a fake index is generated based of the now > > > > > recognized duration: > > > > > > > > > > samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf > > > > > > > > > > but I guess the file is wrong because clip wrapped UL is used when > > > > > the > > > > > file seems frame wrapped instead? > > > > > > > > Nice finding! I can confirm this, it is actually clip wrapped looking > > > > at the mxf dump from bmx. These samples are pretty outdated anyway :D > > > > Unfortunately i don't have enough insight on the internals yet on the > > > > topic about adding/freeing/deleting metadata sets :-( > > > > > > FRAME Wrapped it is, not clip wrapped :rolleyes: > > > ___ > > > 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". > > > > Anything left that needs investigation/correction on this one? ...it > > looks to be a very nice addition so far. > > I was waiting for some comments from Tomas, to see if he has a preference, > but I guess I will apply in a few days if no further comments are > received. I don't have much more to add. We don't have to support every broken file, and this patch beings us more in line with what's correct, as far as I can tell. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] mxf : allow using codecs RAWVIDEO and V210
> -static int64_t mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const
> UID key)
> +static int64_t mxf_write_generic_desc(AVFormatContext *s, AVStream *st,
> const UID key)
> {
> MXFStreamContext *sc = st->priv_data;
> AVIOContext *pb = s->pb;
> @@ -1115,7 +1133,7 @@ static int64_t mxf_write_cdci_common(AVFormatContext
> *s, AVStream *st, const UID
> const MXFCodecUL *color_primaries_ul;
> const MXFCodecUL *color_trc_ul;
> const MXFCodecUL *color_space_ul;
> -int64_t pos = mxf_write_generic_desc(s, st, key);
> +int64_t pos = mxf_write_file_desc(s, st, key);
> uint8_t *side_data;
>
> color_primaries_ul = mxf_get_codec_ul_by_id(ff_mxf_color_primaries_uls,
> st->codecpar->color_primaries);
> @@ -1131,6 +1149,8 @@ static int64_t mxf_write_cdci_common(AVFormatContext
> *s, AVStream *st, const UID
> if (!stored_width)
> stored_width = (st->codecpar->width+15)/16*16;
>
> +// TODO: V210 ==> Stored Width shall be a multiple of 48.
This needs to be fixed before this can be pushed. I don't want
incorrect code in master.
> mxf_write_local_tag(s, 4, 0x3203);
> avio_wb32(pb, stored_width);
> @@ -1432,7 +1486,7 @@ static int64_t
> mxf_write_generic_sound_common(AVFormatContext *s, AVStream *st,
> AVIOContext *pb = s->pb;
> MXFContext *mxf = s->priv_data;
> int show_warnings = !mxf->footer_partition_offset;
> -int64_t pos = mxf_write_generic_desc(s, st, key);
> +int64_t pos = mxf_write_file_desc(s, st, key);
>
> if (s->oformat == _mxf_opatom_muxer) {
> mxf_write_local_tag(s, 8, 0x3002);
> @@ -2348,6 +2402,42 @@ static int mxf_parse_h264_frame(AVFormatContext *s,
> AVStream *st,
> return 1;
> }
>
> +static int mxf_parse_raw_frame(AVFormatContext *s, AVStream *st,
> + AVPacket *pkt, MXFIndexEntry *e)
> +{
> +MXFContext *mxf = s->priv_data;
> +MXFStreamContext *sc = st->priv_data;
> +const MXFCodecUL* uls = ff_mxf_pixel_format_uls;
> +const char* pixelLayoutData = NULL;
> +int format;
> +
> +if (mxf->header_written)
> +return 1;
> +
> +format = st->codecpar->format;
> +
> +if(ff_mxf_find_pixel_layout(, format) < 0)
> +{
> +while (uls->uid[0]) {
> +if (format == uls->id) {
> +sc->codec_ul = >uid;
> +break;
> +}
> +uls++;
> +}
Don't we have a function for these kinds of lookups?
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] Scaling PAL8 images with alpha
fre 2021-09-24 klockan 10:30 + skrev Soft Works: > Hi, > > for a new filter, I want to rescale PAL8 subtitle bitmaps where the > palette includes > colors with alpha. Isn't this typically done at the player level? What's your use case? > > From what I’ve seen, swscale doesn’t support PAL8-to-PAL8, only PAL8- > to-BGR8 > which doesn’t support alpha and the palette is fixed with 256 entries > defined by > convention, while I would ideally like to be able to allow the > following: > > - constrain the output to use the same palette as the input Nearest neighbor scaling should be able to do this. > - adaptively quantize it to a palette with a configurable number of > colors Palette generation is typically an offline process, via palettegen as I said on IRC /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 14/14] avcodec/elbg: Mark ELBGContext as being unaliased by using av_restrict
mån 2021-09-20 klockan 23:18 +0200 skrev Andreas Rheinhardt: > This improves performance: For msvideo1, the performance improved by > 4.8% when encoding the sample from the fate-vsynth1-msvideo1 test; > when encoding the sample from fate-vsynth1-cinepak, performance > improved by 2%. The compiler user was GCC 10 and the calls to encode2 > have been timed. Median wall time for three cinepak runs went from 1m36,455s to 1m35,251s. Not a huge difference. Not worse anyway. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxf: support MCA audio information
tor 2021-09-23 klockan 00:02 +0200 skrev Marton Balint:
On Fri, 17 Sep 2021, Marc-Antoine Arnaud wrote:
> ---
> libavformat/mxf.h | 1 +
> libavformat/mxfdec.c | 277
> ++-
> 2 files changed, 272 insertions(+), 6 deletions(-)
I guess the questionable part of this patch is the internal reordering
of
audio channels. This might or might not be what the user expects.
Can we signal this and have the ffmpeg CLI automagically insert a
channel reordering filter? That way someone wanting to speed the
reordering up could write vectorized code for it
>
> +static int mxf_read_mca_sub_descriptor(void *arg, AVIOContext *pb,
> int tag, int size, UID uid, int64_t klv_offset)
> +{
> + MXFMCASubDescriptor *mca_sub_descriptor = arg;
> +
> + if (IS_KLV_KEY(uid, mxf_mca_label_dictionnary_id)) {
> + avio_read(pb, mca_sub_descriptor->mca_label_dictionnary_id,
> 16);
> + }
> + if (IS_KLV_KEY(uid, mxf_mca_link_id)) {
> + avio_read(pb, mca_sub_descriptor->mca_link_id, 16);
> + }
> + if (IS_KLV_KEY(uid, mxf_soundfield_group_link_id)) {
> + avio_read(pb, mca_sub_descriptor->mca_group_link_id, 16);
> + }
You don't have to open braces for single-line blocks, ffmpeg tends to
follow this style, so preferably you should too.
I actually prefer braces always. I've been bitten by lack of braces at
least once.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
mån 2021-10-11 klockan 18:32 +0200 skrev Marc-Antoine Arnaud: > --- > libavformat/mxf.h | 1 + > libavformat/mxfdec.c | 276 > ++- > 2 files changed, 271 insertions(+), 6 deletions(-) Did we reach a consensus on this? While I think signalling ffmpeg to create a remap filter is a good idea, I don't know whether we have any mechanism for that at the moment. If we don't then perhaps that shouldn't hold up this patch? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
mån 2021-10-18 klockan 17:06 +0200 skrev Marc-Antoine Arnaud:
>
> +static int mxf_audio_remapping(int* channel_ordering, uint8_t* data,
> int size, int sample_size, int channels)
> +{
> + int sample_offset = channels * sample_size;
> + int number_of_samples = size / sample_offset;
> + uint8_t* tmp = av_malloc(sample_offset);
You could avoid this allocation using
uint8_t tmp[FF_SANE_NB_CHANNELS*4];
since mxfdec only handles up to 32-bit PCM. Maybe *8 if someone down
the line decides to add support for 64-bit PCM. Not that I understand
why anyone would ever use that.. Don't think SMPTE defines a UL for it.
> + uint8_t* data_ptr = data;
you can just use data
> +
> + if (!tmp)
> + return AVERROR(ENOMEM);
> +
> + for (int sample = 0; sample < number_of_samples; ++sample) {
> + memcpy(tmp, data_ptr, sample_offset);
> +
> + for (int channel = 0; channel < channels; ++channel) {
> + for (int sample_index = 0; sample_index < sample_size;
> ++sample_index) {
> + data_ptr[sample_size * channel_ordering[channel] +
> sample_index] = tmp[sample_size * channel + sample_index];
> + }
why not memcpy()? Should get inlined by any decent compiler I think
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/9] avcodec/zmbvenc: Remove redundant pixel format check
tis 2021-09-28 klockan 16:41 +0200 skrev Andreas Rheinhardt: > ff_encode_preinit() already checked the pixel format via > AVCodec.pix_fmts. > > Signed-off-by: Andreas Rheinhardt > --- > libavcodec/zmbvenc.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/libavcodec/zmbvenc.c b/libavcodec/zmbvenc.c > index b431476241..8efdbc963e 100644 > --- a/libavcodec/zmbvenc.c > +++ b/libavcodec/zmbvenc.c > @@ -347,9 +347,6 @@ static av_cold int encode_init(AVCodecContext > *avctx) > c->fmt = ZMBV_FMT_32BPP; > c->bypp = 4; > break; > - default: > - av_log(avctx, AV_LOG_INFO, "unsupported pixel format\n"); > - return AVERROR(EINVAL); Sounds OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs
tis 2021-11-30 klockan 10:22 +0100 skrev Nicolas Gaullier:
Fix GC container ul.
Fix GC element type both for the generic case and for OPAtom.
Thanks to Philip de Nier
for checking the values, especially for OPAtom.
---
libavformat/mxfenc.c | 8 ++--
tests/ref/lavf/mxf_opatom | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index fcd9afda2a..38de3d1ab5 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -32,6 +32,7 @@
* SMPTE 379M MXF Generic Container
* SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
* SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic
Container
+ * SMPTE ST2019-4 Mapping VC-3 Coding Units into the MXF Generic
Container
* SMPTE RP210: SMPTE Metadata Dictionary
* SMPTE RP224: Registry of SMPTE Universal Labels
*/
@@ -181,8 +182,8 @@ static const MXFContainerEssenceEntry
mxf_essence_container_uls[] = {
{
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00,0
x00,0x00 },
mxf_write_cdci_desc },
// DNxHD
- { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11,0
x01,0x00 },
- {
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0
x05,0x00 },
+ { {
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11,0
x01,0x00 },
+ {
0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01,0
x0C,0x00 },
{
0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01,0
x00,0x00 },
Please add a reference to the relevant SMPTE document in the comment,
or perhaps at the list of references at the start of the file
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxf: support MCA audio information
tis 2021-12-21 klockan 11:44 +0100 skrev Marc-Antoine ARNAUD: > Le ven. 17 déc. 2021 à 19:12, Marton Balint a écrit : > > > > > > > On Fri, 17 Dec 2021, Marc-Antoine ARNAUD wrote: > > > > > Hi all, > > > > > > Can I have an update on this patch submission ? > > > Is something required to be done before it can be merged ? > > > > New channel layout API is on its way, which makes in-demuxer > > channel > > reordering uneeded. Therefore the reordering option should not be > > added > > as it is in this patch. I can rework the patch after the channel > > layout > > API is in. (should happen in a couple of weeks at most). > > > > Regards, > > Marton > > > > So it will happen only after the release 5 of FFMpeg right ? > > Is it possible to merge it, and we can rework it after the new API is > released ? > Patches are related to IMF (new format) patches, and if FFmpeg can > accept > IMF without MCA support it will generate a lot of errors in audio > mapping. > So even if it's not performant for now, is it possible to imagine to > merge > patches and rework after ? I suspect this might create problems for the people writing the reordering API /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 13/15] avformat/mux, mxfenc: Don't use sizeof(AVPacket)
tor 2021-12-16 klockan 02:29 +0100 skrev Andreas Rheinhardt:
> This removes the last usage of sizeof(AVPacket) in the generic
> muxing code.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mux.c | 8 +---
> libavformat/mxfenc.c | 8 ++--
> 2 files changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index b9c4abb9cf..0500f636de 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -1014,15 +1014,9 @@ int
> ff_interleave_packet_per_dts(AVFormatContext *s, AVPacket *pkt,
> AVStream *const st = s->streams[pktl->pkt.stream_index];
> FFStream *const sti = ffstream(st);
>
> - *pkt = pktl->pkt;
> -
> - si->packet_buffer = pktl->next;
> - if (!si->packet_buffer)
> - si->packet_buffer_end = NULL;
> -
> if (sti->last_in_packet_buffer == pktl)
> sti->last_in_packet_buffer = NULL;
> - av_freep();
> + avpriv_packet_list_get(>packet_buffer,
> >packet_buffer_end, pkt);
>
> return 1;
> } else {
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 00bbe58149..7635e183d0 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -3125,14 +3125,10 @@ static int
> mxf_interleave_get_packet(AVFormatContext *s, AVPacket *out, int flus
> pktl = si->packet_buffer;
> }
>
> - *out = pktl->pkt;
> - av_log(s, AV_LOG_TRACE, "out st:%d dts:%"PRId64"\n",
> (*out).stream_index, (*out).dts);
> - si->packet_buffer = pktl->next;
> if (ffstream(s->streams[pktl->pkt.stream_index])-
> >last_in_packet_buffer == pktl)
> ffstream(s->streams[pktl->pkt.stream_index])-
> >last_in_packet_buffer = NULL;
> - if (!si->packet_buffer)
> - si->packet_buffer_end = NULL;
> - av_freep();
> + avpriv_packet_list_get(>packet_buffer,
> >packet_buffer_end, out);
> + av_log(s, AV_LOG_TRACE, "out st:%d dts:%"PRId64"\n", out-
> >stream_index, out->dts);
> return 1;
> } else {
> out:
Looks like this makes the code simpler, which is fine by me
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] GitHub Integration
ons 2021-12-22 klockan 23:24 + skrev Soft Works: > Hi, > > holidays are approaching and I got a little present for all of you > even though it won’t be something for everybody. > > A while ago I had committed to prepare a test setup for integrating > GitHub in a similar way as the Git developers are doing. > > For those who missed it or don’t remember the outcome: > https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg123329.html > https://www.mail-archive.com/ffmpeg-devel@ffmpeg.org/msg123288.html > > Before someone’s temperature might start rising, I want to repeat > some important corner points: > > - it's not a migration nor the start of a migration > - nothing changes, it doesn't affect anybody's way of working > it doesn't replace anything > - it's an add-on that one could use or not > > Basic functionality is that it allows to submit patches to the ML > through GitHub pull requests. This sounds like something that will cause problems in the long run. Github will inevitably be brought into the project's workflow. People will start submitting tickets on Github rather than our trac. And so on. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxf: support MCA audio information
tis 2021-12-21 klockan 21:24 +0100 skrev Marton Balint: > > > On Tue, 21 Dec 2021, Tomas Härdin wrote: > > > tis 2021-12-21 klockan 11:44 +0100 skrev Marc-Antoine ARNAUD: > > > Le ven. 17 déc. 2021 à 19:12, Marton Balint a > > > écrit : > > > > > > > > > > > > > > > On Fri, 17 Dec 2021, Marc-Antoine ARNAUD wrote: > > > > > > > > > Hi all, > > > > > > > > > > Can I have an update on this patch submission ? > > > > > Is something required to be done before it can be merged ? > > > > > > > > New channel layout API is on its way, which makes in-demuxer > > > > channel > > > > reordering uneeded. Therefore the reordering option should not > > > > be > > > > added > > > > as it is in this patch. I can rework the patch after the > > > > channel > > > > layout > > > > API is in. (should happen in a couple of weeks at most). > > > > > > > > Regards, > > > > Marton > > > > > > > > > > So it will happen only after the release 5 of FFMpeg right ? > > Not sure. There were people who wanted the merge the channel layout > api > before the release. > > > > > > > Is it possible to merge it, and we can rework it after the new > > > API is > > > released ? > > > Patches are related to IMF (new format) patches, and if FFmpeg > > > can > > > accept > > > IMF without MCA support it will generate a lot of errors in audio > > > mapping. > > > So even if it's not performant for now, is it possible to imagine > > > to > > > merge > > > patches and rework after ? > > > > I suspect this might create problems for the people writing the > > reordering API > > It is not matter of performance, we should not introduce a hack such > as > reordering PCM channels in a demuxer if there is a better solution on > the > horizon. > > If this is urgent, we could merge it without reordering support. I think we can trust our users to maintain their own forks if they need special hacks /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 03/17] fate/mxf: Add tests for H.264 remuxing
ons 2021-11-10 klockan 14:52 +0100 skrev Andreas Rheinhardt: > Tomas Härdin: > > tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > > > These tests exhibit two bugs: Instead of using the in-band > > > extradata > > > the demuxer makes up some extradata designed for AVC intra tracks > > > that lack in-band extradata; these files are nevertheless > > > decodable > > > because of the in-band extradata. Furthermore, the frame > > > reordering > > > is lost. > > > > > > Signed-off-by: Andreas Rheinhardt > > > > > > --- > > > tests/fate/mxf.mak | 22 ++- > > > tests/ref/fate/mxf-remux-h264 | 37 ++ > > > tests/ref/fate/mxf-remux-xavc | 71 > > > +++ > > > 3 files changed, 128 insertions(+), 2 deletions(-) > > > create mode 100644 tests/ref/fate/mxf-remux-h264 > > > create mode 100644 tests/ref/fate/mxf-remux-xavc > > > > > > diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak > > > index f96f4a429b..58a697cd86 100644 > > > --- a/tests/fate/mxf.mak > > > +++ b/tests/fate/mxf.mak > > > @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, > > > PRORES_DECODER > > > MXF_MUXER) \ > > > += fate-mxf-remux-applehdr10 > > > fate-mxf-remux-applehdr10: CMD = transcode mxf > > > $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "- > > > map > > > 0 -c copy" "-c copy -t 0.3" "" "-show_entries > > > format_tags:stream_side_data_list:stream=index,codec_name,codec_t > > > ag:s > > > tream_tags" > > > > > > +# Tests muxing H.264, in particular automatic insertion of > > > h264_mp4toannexb. > > > +# FIXME: The timestamps of the demuxed file are not properly > > > reordered. > > > +# Furthermore the extradata is wrong: It is one of the AVC intra > > > SPS/PPS; > > > +# decoding only works due to in-band extradata. > > > > Is this a problem with the samples or the code? Or both? > > > > The extradata issue is due to a bug in the demuxer: It always adds > extradata for H.264 instead of letting extract_extradata_bsf extract > it > from the bitstream. See these lines here: > https://github.com/FFmpeg/FFmpeg/blob/447cf537746cd9969674ebbd60411b6093603c59/libavformat/mxfdec.c#L2711-L2718 > The solution for this is of course to detect whether we are dealing > with > AVC intra data and only adding the extradata in this case. What is > the > right way to check for this? Is it the presence of the H.264 ul in > mxf_intra_only_picture_essence_coding_uls? (Another way would be to > check whether the first packet has extradata, but this is probably > more > involved (we would basically have to reimplement/reuse a part of > avformat_find_stream_info() (the part that deals with > extract_extradata) > ourselves).) > I don't know whether the timestamps are due to a bug in the muxer or > the > demuxer, but the timestamps the muxer receives are properly > reordered. Answers to questions like these are in the official SMPTE mapping documents. We should not invent our own hacks. Typically a UL is used to signal this, yes. For AVC-Intra, PPS and SPS seem to be implicit, hence ff_generate_avci_extradata(). For other AVC/H.264 mappings it might be that there's a metadata KLV that stores the extradata, or extradata are to be parsed from the essence. Again, the mapping document should specify this. Do *not* add hacks that try to autodetect this stuff. That just encourages even more hacks and deviations from the standards. Hacks beget hacks. At some point we might have to go through mxfdec.c and limit already existing hacks to only apply to files written by specific MXF encoders. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/17] avformat/mxfenc: Auto-insert h264_mp4toannexb BSF if needed
ons 2021-11-10 klockan 14:21 +0100 skrev Andreas Rheinhardt: > Tomas Härdin: > > tis 2021-11-09 klockan 22:07 +0100 skrev Andreas Rheinhardt: > > > Tomas Härdin: > > > > tis 2021-11-09 klockan 18:34 +0100 skrev Andreas Rheinhardt: > > > > > The mxf and mxf_opatom muxer expect H.264 in Annex B format. > > > > > > > > > > Signed-off-by: Andreas Rheinhardt > > > > > > > > > > --- > > > > > The check here is taken from mpegtsenc. > > > > > > > > You didn't think to make both muxers share code instead of > > > > copy- > > > > pasting? > > > > > > > > > > Well, I can share it. The problem is just that I didn't really > > > know > > > where it belongs: mux.c? utils.c? A new file? > > > > A new file probably, say libavformat/annexb.c > > > > Do we ever need to be able to do this kind of stuff in lavc? In > > that > > case it could maybe live there. But that again increases coupling > > between the two libs.. > > > > lavc does not have AVStreams or AVFormatContexts, so sharing code > would > be absolutely impossible anyway. But one can share ideas: Using a bsf > to > prepare data for a decoder is also done for certain decoders; see > AVCodec.bsfs. These bitstream filters are applied unconditionally and > are therefore supposed to detect themselves whether they should do > something or whether the data already has the desired form. I am > unsure > whether this is a better approach; doing the same in lavf would add a > problem that lavc does not have: There would be a copy of every > non-refcounted packet when using av_write_frame(). Just noting here that it might be that we shouldn't even insert a BSF. See my comment on patch 03. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
lör 2021-10-30 klockan 10:28 -0400 skrev Ronald S. Bultje: > Hi, > > On Sat, Oct 30, 2021 at 4:57 AM Tomas Härdin > wrote: > > > Maybe we should upgrade to C11 then? This gives us access to more > > useful language features. Type-generic expressions look very useful > > > > https://stackoverflow.com/a/7005988 (same thread, further down) > appears to > suggest the exact same literal wording exists in C99. No upgrade is > necessary. Ah. Well disregard my ramblings then (: /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 21:17 -0400 skrev Ronald S. Bultje: > Hi Thomas, > > On Fri, Oct 29, 2021 at 9:12 AM Tomas Härdin > wrote: > > > tor 2021-10-28 klockan 21:09 +0200 skrev Paul B Mahol: > > > + const uint16_t *src = (const uint16_t *)ssrc; > > > > This is not -fstrict-aliasing safe > > > > I don't believe that is correct. It's true we're not allowed to cast > between two clashing types to access the same pointer (memory > location), > but the C standard would appear to make an exception for byte types. > > Quoted from https://stackoverflow.com/a/99010/4726410 because I'm too > lazy > to dig through the standard: > > "The types that C 2011 6.5 7 allows an lvalue to access are: > - a type compatible with the effective type of the object, > - a qualified version of a type compatible with the effective type of > the > object, > - a type that is the signed or unsigned type corresponding to the > effective > type of the object, > - a type that is the signed or unsigned type corresponding to a > qualified > version of the effective type of the object, > - an aggregate or union type that includes one of the aforementioned > types > among its members (including, recursively, a member of a subaggregate > or > contained union), or > - a character type." > > uint8_t is a variant of the character (aka byte) type, and so the > cast > would not seem to violate strict aliasing rules. Maybe we should upgrade to C11 then? This gives us access to more useful language features. Type-generic expressions look very useful /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
tor 2021-10-28 klockan 21:09 +0200 skrev Paul B Mahol:
>
> +FRAMESYNC_DEFINE_CLASS(wpsnr, WPSNRContext, fs);
> +
> +#define COMPUTE_HX(type, stype, depth) \
> +static void compute_hx##depth(const uint8_t *ssrc, \
> + int linesize, \
> + int w, int h, \
> + uint16_t *dstp, \
> + int dst_linesize) \
> +{ \
> + const type *src = (const type *)ssrc; \
> + stype *dst = (stype *)dstp; \
> + \
> + linesize /= (depth / 8); \
Can linesize ever be odd? Probably not, so this should be fine.
> +static double get_hx(const uint8_t *src, int linesize, int w, int h)
> +{
> + int64_t sum = 0;
> +
> + for (int y = 0; y < h; y++) {
> + for (int x = 0; x < w; x++) {
> + sum += 12 * src[x] -
> + 2 * (src[x-1] + src[x+1] +
> + src[x + linesize] +
> + src[x - linesize]) -
> + 1 * (src[x - 1 - linesize] +
> + src[x + 1 - linesize] +
> + src[x - 1 + linesize] +
> + src[x + 1 + linesize]);
> + }
> +
> + src += linesize;
> + }
> +
> + return fabs(sum * 0.25);
> +}
> +
> +static double get_hx16(const uint8_t *ssrc, int linesize, int w, int
> h)
> +{
> + const uint16_t *src = (const uint16_t *)ssrc;
This is not -fstrict-aliasing safe
> + int64_t sum = 0;
> +
> + linesize /= 2;
> +
> + for (int y = 0; y < h; y++) {
> + for (int x = 0; x < w; x++) {
> + sum += 12 * src[x] -
> + 2 * (src[x-1] + src[x+1] +
> + src[x + linesize] +
> + src[x - linesize]) -
> + 1 * (src[x - 1 - linesize] +
> + src[x + 1 - linesize] +
> + src[x - 1 + linesize] +
> + src[x + 1 + linesize]);
> + }
> +
> + src += linesize;
> + }
> +
> + return fabs(sum * 0.25);
> +}
Why not use the same kind of templatization for these as compute_hx*?
> +
> +static double get_sd(const uint8_t *ref, int ref_linesize,
> + const uint8_t *main, int main_linesize,
> + int w, int h)
> +{
> + int64_t sum = 0;
> +
> + for (int y = 0; y < h; y++) {
> + for (int x = 0; x < w; x++)
> + sum += pow_2(ref[x] - main[x]);
> + ref += ref_linesize;
> + main += main_linesize;
> + }
> +
> + return sum;
> +}
> +
> +static double get_sd16(const uint8_t *rref, int ref_linesize,
> + const uint8_t *mmain, int main_linesize,
> + int w, int h)
> +{
> + const uint16_t *ref = (const uint16_t *)rref;
> + const uint16_t *main = (const uint16_t *)mmain;
> + int64_t sum = 0;
> +
> + ref_linesize /= 2;
> + main_linesize /= 2;
> +
> + for (int y = 0; y < h; y++) {
> + for (int x = 0; x < w; x++)
> + sum += pow_2(ref[x] - main[x]);
> + ref += ref_linesize;
> + main += main_linesize;
> + }
> +
> + return sum;
> +}
Same here, and for more functions in the patch it seems so I'm not
going to bother repeating myself any more
> +static void set_meta(AVDictionary **metadata, const char *key, char
> comp, float d)
> +{
> + char value[128];
> + snprintf(value, sizeof(value), "%f", d);
> + if (comp) {
> + char key2[128];
> + snprintf(key2, sizeof(key2), "%s%c", key, comp);
> + av_dict_set(metadata, key2, value, 0);
> + } else {
> + av_dict_set(metadata, key, value, 0);
> + }
> +}
We should probably add av_dict_set_* for int, double etc at some point
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol:
> On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin
> wrote:
> >
> > > +static double get_hx(const uint8_t *src, int linesize, int w,
> > > int h)
> > > +{
> > > + int64_t sum = 0;
> > > +
> > > + for (int y = 0; y < h; y++) {
> > > + for (int x = 0; x < w; x++) {
> > > + sum += 12 * src[x] -
> > > + 2 * (src[x-1] + src[x+1] +
> > > + src[x + linesize] +
> > > + src[x - linesize]) -
> > > + 1 * (src[x - 1 - linesize] +
> > > + src[x + 1 - linesize] +
> > > + src[x - 1 + linesize] +
> > > + src[x + 1 + linesize]);
> > > + }
> > > +
> > > + src += linesize;
> > > + }
> > > +
> > > + return fabs(sum * 0.25);
> > > +}
> > > +
> > > +static double get_hx16(const uint8_t *ssrc, int linesize, int w,
> > > int
> > > h)
> > > +{
> > > + const uint16_t *src = (const uint16_t *)ssrc;
> >
> > This is not -fstrict-aliasing safe
> >
>
> How so? I get no warnings at all, and similar is used everywhere
> else.
Then those places should be fixed. We have macros like AV_RB16() for a
reason. That gcc doesn't warn about this doesn't mean it isn't free to
assume ssrc and src points to different non-overlapping regions of
memory.
We could disable strict aliasing, but that will likely hurt performance
of other parts of the code.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] mfxenc add jpeg2000 frame field interlacing support
Looks like you messed up the subject. You need two newlines between the
title of the patch and the description.
This patch also mixes a lot of different changes. Please split it up.
The bigger a patch is the more rounds of review it tends to have to go
through.
> + { 0x840B,
> {0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0a,0x04,0x01,0x06,0x03,0x0B,0x0
> 0,0x00,0x00}}, /* 8+3n bytes : Array of picture components where each
> component comprises 3 bytes named Ssizi, XRSizi, YRSizi The array of
> 3-byte groups is preceded by the array header comprising a 4-byte
> value of the number of components followed by a 4-byte value of �3�.
> */
some kind of encoding problem in the comment
> @@ -843,7 +886,28 @@ static void mxf_write_track(AVFormatContext *s,
> AVStream *st, MXFPackage *packag
>
> mxf_write_metadata_key(pb, 0x013b00);
> PRINT_KEY(s, "track key", pb->buf_ptr - 16);
> - klv_encode_ber_length(pb, 80);
> +
> + if (st->codecpar) {
> + static const char * pcTrackName_Video = "Picture" ;
> + static const char * pcTrackName_Audio = "Sound" ;
> + static const char * pcTrackName_Anc = "Ancillary" ;
static is redundant here
> + if ( st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO )
> + {
> + //TrackName Video
> + klv_encode_ber_length(pb, 80 +
> mxf_utf16_local_tag_length(pcTrackName_Video));
> + mxf_write_local_tag_utf16(s, 0x4802 ,
> pcTrackName_Video);
> + } else if ( st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO )
> {
> + //TrackName Audio
> + klv_encode_ber_length(pb, 80 +
> mxf_utf16_local_tag_length(pcTrackName_Audio));
> + mxf_write_local_tag_utf16(s, 0x4802, pcTrackName_Audio);
> + } else {
> + //TrackName Ancillary
> + klv_encode_ber_length(pb, 80 +
> mxf_utf16_local_tag_length(pcTrackName_Anc));
> + mxf_write_local_tag_utf16(s, 0x4802, pcTrackName_Anc);
> + }
> + } else {
> + klv_encode_ber_length(pb, 80);
> + }
Is this hunk really necessary?
> @@ -1327,6 +1420,182 @@ static int64_t
> mxf_write_cdci_common(AVFormatContext *s, AVStream *st, const UID
> return pos;
> }
>
> +static int64_t mxf_write_jpeg2000_common(AVFormatContext *s,
> AVStream *st, const UID key)
> +{
> + MXFStreamContext *sc = st->priv_data;
> + MXFContext *mxf = s->priv_data;
> + AVIOContext *pb = s->pb;
> + int stored_width = st->codecpar->width;
> + int stored_height = st->codecpar->height;
> + int display_height;
> + int f1, f2;
> + UID transfer_ul = {0};
> + int64_t pos = mxf_write_generic_desc(s, st, key);
> + get_trc(transfer_ul, st->codecpar->color_trc);
Return value of get_trc() is not checked. Not sure if invalid values
can ever get in here.
> +
> + mxf_write_local_tag(s, 4, 0x3203);
> + avio_wb32(pb, stored_width);
> + mxf_write_local_tag(s, 4, 0x3202);
> + avio_wb32(pb, stored_height);
> +
> + //Sampled width
> + mxf_write_local_tag(s, 4, 0x3205);
> + avio_wb32(pb, st->codecpar->width);
> +
> + //Samples height
> + mxf_write_local_tag(s, 4, 0x3204);
> + avio_wb32(pb, st->codecpar->height);
Why use stored_* and codecpar->*? Just use codecpar->* in both places.
> +
> + //Sampled X Offset
> + mxf_write_local_tag(s, 4, 0x3206);
> + avio_wb32(pb, 0);
> +
> + //Sampled Y Offset
> + mxf_write_local_tag(s, 4, 0x3207);
> + avio_wb32(pb, 0);
> +
> + mxf_write_local_tag(s, 4, 0x3209);
> + avio_wb32(pb, st->codecpar->width);
> +
> + if (st->codecpar->height == 608) // PAL + VBI
> + display_height = 576;
> + else if (st->codecpar->height == 512) // NTSC + VBI
> + display_height = 486;
> + else
> + display_height = st->codecpar->height;
> +
> + mxf_write_local_tag(s, 4, 0x3208);
> + avio_wb32(pb, display_height);
> +
> + // display X offset
> + mxf_write_local_tag(s, 4, 0x320A);
> + avio_wb32(pb, 0);
> +
> + //display Y offset
> + mxf_write_local_tag(s, 4, 0x320B);
> + avio_wb32(pb, (st->codecpar->height - display_height));
Would be better if we could get this information via metadata instead
of adding hacks to the muxer
> + // // Padding Bits
> + // mxf_write_local_tag(s, 2, 0x3307);
> + // avio_wb16(pb, 0);
Stray dead code
> + // video line map
> + {
> + int _field_height = (mxf->mxf_j2kinterlace) ? (2*st-
> >codecpar->height) : st->codecpar->height;
> +
> + if (st->codecpar->sample_aspect_ratio.num && st->codecpar-
> >sample_aspect_ratio.den) {
> + AVRational _ratio = av_mul_q(st->codecpar-
> >sample_aspect_ratio,
> + av_make_q(st->codecpar->width, st-
> >codecpar->height));
> + sc->aspect_ratio = _ratio;
> +
> + switch (_field_height) {
> + case 576: f1 = 23; f2 = st->codecpar->codec_id ==
> AV_CODEC_ID_DVVIDEO ? 335 : 336; break;
> + case 608: f1 = 7; f2 = 320; break;
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
ons 2021-10-27 klockan 15:50 +0200 skrev Marc-Antoine Arnaud:
> +if(channel_ordering_ptr->service_type !=
> AV_AUDIO_SERVICE_TYPE_NB) {
> +ast = (enum
> AVAudioServiceType*)av_stream_new_side_data(st,
> AV_PKT_DATA_AUDIO_SERVICE_TYPE, sizeof(*ast));
This needs a check for ast == NULL
> +static int mxf_audio_remapping(int* channel_ordering, uint8_t* data,
> int size, int sample_size, int channels)
> +{
> +int sample_offset = channels * sample_size;
> +int number_of_samples = size / sample_offset;
> +uint8_t tmp[FF_SANE_NB_CHANNELS * 4];
> +uint8_t* data_ptr = data;
> +
> +if (!tmp)
> +return AVERROR(ENOMEM);
> +
> +for (int sample = 0; sample < number_of_samples; ++sample) {
> +memcpy(tmp, data_ptr, sample_offset);
> +
> +for (int channel = 0; channel < channels; ++channel) {
> +for (int sample_index = 0; sample_index < sample_size;
> ++sample_index) {
> +data_ptr[sample_size * channel_ordering[channel] +
> sample_index] = tmp[sample_size * channel + sample_index];
> +}
What I meant with my previous comment on this is that the innermost
loop can be replaced with memcpy(), making the code simpler.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 17:00 +0200 skrev Paul B Mahol:
> On Fri, Oct 29, 2021 at 4:46 PM Tomas Härdin
> wrote:
>
> > fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol:
> > > On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin
> > > wrote:
> > > >
> > > > > +static double get_hx(const uint8_t *src, int linesize, int
> > > > > w,
> > > > > int h)
> > > > > +{
> > > > > + int64_t sum = 0;
> > > > > +
> > > > > + for (int y = 0; y < h; y++) {
> > > > > + for (int x = 0; x < w; x++) {
> > > > > + sum += 12 * src[x] -
> > > > > + 2 * (src[x-1] + src[x+1] +
> > > > > + src[x + linesize] +
> > > > > + src[x - linesize]) -
> > > > > + 1 * (src[x - 1 - linesize] +
> > > > > + src[x + 1 - linesize] +
> > > > > + src[x - 1 + linesize] +
> > > > > + src[x + 1 + linesize]);
> > > > > + }
> > > > > +
> > > > > + src += linesize;
> > > > > + }
> > > > > +
> > > > > + return fabs(sum * 0.25);
> > > > > +}
> > > > > +
> > > > > +static double get_hx16(const uint8_t *ssrc, int linesize,
> > > > > int w,
> > > > > int
> > > > > h)
> > > > > +{
> > > > > + const uint16_t *src = (const uint16_t *)ssrc;
> > > >
> > > > This is not -fstrict-aliasing safe
> > > >
> > >
> > > How so? I get no warnings at all, and similar is used everywhere
> > > else.
> >
> > Then those places should be fixed. We have macros like AV_RB16()
> > for a
> > reason. That gcc doesn't warn about this doesn't mean it isn't free
> > to
> > assume ssrc and src points to different non-overlapping regions of
> > memory.
> >
> >
> That is sub optimal and unacceptable solution.
Did you do measurements to come to this conclusion?
> Review remark ignored.
Undefined behavior is *not* acceptable. If you want this file
specifically to be compiled with strict aliasing disabled then you must
at the very least change the build system accordingly
Type punning via union seems to be defined as of C99, so that should
also be acceptable
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 19:17 +0200 skrev Paul B Mahol:
> On Fri, Oct 29, 2021 at 6:59 PM Tomas Härdin
> wrote:
>
> > fre 2021-10-29 klockan 17:00 +0200 skrev Paul B Mahol:
> > > On Fri, Oct 29, 2021 at 4:46 PM Tomas Härdin
> > > wrote:
> > >
> > > > fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol:
> > > > > On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin <
> > > > > tjop...@acc.umu.se>
> > > > > wrote:
> > > > > >
> > > > > > > +static double get_hx(const uint8_t *src, int linesize,
> > > > > > > int
> > > > > > > w,
> > > > > > > int h)
> > > > > > > +{
> > > > > > > + int64_t sum = 0;
> > > > > > > +
> > > > > > > + for (int y = 0; y < h; y++) {
> > > > > > > + for (int x = 0; x < w; x++) {
> > > > > > > + sum += 12 * src[x] -
> > > > > > > + 2 * (src[x-1] + src[x+1] +
> > > > > > > + src[x + linesize] +
> > > > > > > + src[x - linesize]) -
> > > > > > > + 1 * (src[x - 1 - linesize] +
> > > > > > > + src[x + 1 - linesize] +
> > > > > > > + src[x - 1 + linesize] +
> > > > > > > + src[x + 1 + linesize]);
> > > > > > > + }
> > > > > > > +
> > > > > > > + src += linesize;
> > > > > > > + }
> > > > > > > +
> > > > > > > + return fabs(sum * 0.25);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static double get_hx16(const uint8_t *ssrc, int
> > > > > > > linesize,
> > > > > > > int w,
> > > > > > > int
> > > > > > > h)
> > > > > > > +{
> > > > > > > + const uint16_t *src = (const uint16_t *)ssrc;
> > > > > >
> > > > > > This is not -fstrict-aliasing safe
> > > > > >
> > > > >
> > > > > How so? I get no warnings at all, and similar is used
> > > > > everywhere
> > > > > else.
> > > >
> > > > Then those places should be fixed. We have macros like
> > > > AV_RB16()
> > > > for a
> > > > reason. That gcc doesn't warn about this doesn't mean it isn't
> > > > free
> > > > to
> > > > assume ssrc and src points to different non-overlapping regions
> > > > of
> > > > memory.
> > > >
> > > >
> > > That is sub optimal and unacceptable solution.
> >
> > Did you do measurements to come to this conclusion?
> >
> > > Review remark ignored.
> >
> > Undefined behavior is *not* acceptable. If you want this file
> > specifically to be compiled with strict aliasing disabled then you
> > must
> > at the very least change the build system accordingly
> >
>
> Cite source that can prove this.
The gcc manpage
> Compilers are silent about this.
Irrelevant
> There are hundredths if not thousands of such examples across
> codebase.
This is not a valid excuse. We need less UB in the codebase, not more.
UBs beget CVEs.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
sön 2021-11-07 klockan 12:32 +0100 skrev Marton Balint:
>
> > +
> > + while (channel_ordering_ptr->uid[0]) {
> > + if (IS_KLV_KEY(channel_ordering_ptr->uid,
> > mca_sub_descriptor->mca_label_dictionary_id)) {
>
> You should check if current_channel < desciptor->channels here, and
> if
> not, then warn the user and break out of the loop. Otherwise
> current_channel can grow out of array limits.
>
> It should also be checked that channel_ordering_ptr->index <
> descriptor->channels, and if not, then similarly, warn the user and
> break
> out.
>
> Maybe a hard failure (returning AVERROR_INVALIDDATA) is not
> necessary, to
> allow some slightly invalid metadata?
We should be as strict as we can get away with. Else we encourage
laxness in other implementations.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/17] avformat/mxfenc: Auto-insert h264_mp4toannexb BSF if needed
tis 2021-11-09 klockan 18:34 +0100 skrev Andreas Rheinhardt: > The mxf and mxf_opatom muxer expect H.264 in Annex B format. > > Signed-off-by: Andreas Rheinhardt > --- > The check here is taken from mpegtsenc. You didn't think to make both muxers share code instead of copy- pasting? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 04/17] avformat/mxfenc: Use smaller types to make struct smaller
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfenc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index d1c4d43a50..3b6604d0d6 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2203,9 +2203,9 @@ static int mxf_parse_dv_frame(AVFormatContext
> *s, AVStream *st, AVPacket *pkt)
> static const struct {
> UID uid;
> int frame_size;
> - int profile;
> + uint8_t profile;
> uint8_t interlaced;
> - int intra_only; // 1 or 0 when there are separate UIDs for Long
> GOP and Intra, -1 when Intra/LGOP detection can be ignored
> + int8_t intra_only; // 1 or 0 when there are separate UIDs for
Looks OK, and should work as intended since they're at the end of the
struct.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 02/17] fate/mxf: Add ProRes remux test
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > Also covers writing mastering display metadata. So you're merging the D-10 user comments test and ProRes remuxing? What about remuxing D-10? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 05/17] avformat/mxfenc: Remove redundant check
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> None of the muxers here has the AVFMT_NOSTREAMS flag set,
> so it is checked generically that there are streams.
Didn't know about AVFMT_NOSTREAMS
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfenc.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 3b6604d0d6..fd9e2c4c48 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2507,9 +2507,6 @@ static int mxf_write_header(AVFormatContext *s)
> uint8_t present[FF_ARRAY_ELEMS(mxf_essence_container_uls)] =
> {0};
> int64_t timestamp = 0;
>
> - if (!s->nb_streams)
> - return -1;
Looks OK
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 08/17] avformat/mxfenc: Improve returned error codes
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > Signed-off-by: Andreas Rheinhardt > --- > libavformat/mxfenc.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) Aha, this one does exactly what I just suggested with return values. D'oh! Looks OK of course /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 09/17] avformat/mxfenc: Avoid allocation for timecode track
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfenc.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index cf63340313..aa9857fcff 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -421,6 +421,7 @@ typedef struct MXFContext {
> int track_instance_count; // used to generate MXFTrack uuids
> int cbr_index; ///< use a constant bitrate index
> uint8_t unused_tags[MXF_NUM_TAGS]; ///< local tags that we know
> will not be used
> + MXFStreamContext timecode_track_priv;
> } MXFContext;
>
> static void mxf_write_uuid(AVIOContext *pb, enum MXFMetadataSetType
> type, int value)
> @@ -2712,9 +2713,7 @@ static int mxf_init(AVFormatContext *s)
> mxf->timecode_track = av_mallocz(sizeof(*mxf->timecode_track));
> if (!mxf->timecode_track)
> return AVERROR(ENOMEM);
> - mxf->timecode_track->priv_data =
> av_mallocz(sizeof(MXFStreamContext));
> - if (!mxf->timecode_track->priv_data)
> - return AVERROR(ENOMEM);
> + mxf->timecode_track->priv_data = >timecode_track_priv;
> mxf->timecode_track->index = -1;
>
> return 0;
> @@ -3087,10 +3086,7 @@ static void mxf_deinit(AVFormatContext *s)
>
> av_freep(>index_entries);
> av_freep(>body_partition_offset);
> - if (mxf->timecode_track) {
> - av_freep(>timecode_track->priv_data);
> - av_freep(>timecode_track);
> - }
> + av_freep(>timecode_track);
> }
>
> static int mxf_interleave_get_packet(AVFormatContext *s, AVPacket
> *out, int flush)
Looks OK. We never have more than one timecode track.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 10/17] avformat/mxfdec: Simplify data->hex string conversion
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfdec.c | 13 +++--
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index af9d33f796..4191e82474 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1984,22 +1984,15 @@ static int mxf_uid_to_str(UID uid, char
> **str)
>
> static int mxf_umid_to_str(UID ul, UID uid, char **str)
> {
> - int i;
> char *p;
> p = *str = av_mallocz(sizeof(UID) * 4 + 2 + 1);
> if (!p)
> return AVERROR(ENOMEM);
> snprintf(p, 2 + 1, "0x");
Could use strncpy() while you're at it.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 06/17] avformat/mxfenc: Make init function out of write_header
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > The MXF muxers only write the header after they have received > a packet; the actual write_header function does not write anything. > So make an init function out of it. New API being put to good use. Patch looks OK We could write *some* output if we really wanted to, like the header partition key and klv fill. Not very useful however. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 07/17] avformat/mxfenc: Error out when receiving invalid data
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > (Unless the packet has a size of zero, the packet will run afoul > of the cbr_index check a few lines below.) > > Signed-off-by: Andreas Rheinhardt > --- > libavformat/mxfenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > index a6535eb43f..c20ba9bfca 100644 > --- a/libavformat/mxfenc.c > +++ b/libavformat/mxfenc.c > @@ -2151,7 +2151,7 @@ static int mxf_parse_dv_frame(AVFormatContext > *s, AVStream *st, AVPacket *pkt) > > // Check for minimal frame size > if (pkt->size < 12) > - return -1; > + return 0; Might be nicer to have negative return value mean error, just like everywhere else, and change the code further down accordingly. -1 is a crappy return value for mxf_write_packet() I think. But then the other parsing functions should change at the same time. So let's not hold this patch up with that. Looks OK for now. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 11/17] avformat/mxfenc: Store locally whether DNXHD profile is interlaced
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> It is just a flag per supported CID. So there is no reason to use
> an avpriv function for this purpose.
This is data duplication. Honestly these ULs should probably live in
dnxhddata.c.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfenc.c | 47 ++
> --
> 1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index aa9857fcff..326ec6a7d6 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2036,29 +2036,30 @@ static int
> mxf_parse_prores_frame(AVFormatContext *s, AVStream *st, AVPacket *pk
> }
>
> static const struct {
> - int cid;
> + uint16_t cid;
> + uint8_t interlaced;
> UID codec_ul;
> } mxf_dnxhd_codec_uls[] = {
Not sure if the narrowing of types here does any good. You might need
to put cid and interlaced after codec_ul. On the other hand UID is
uint8_t[] so perhaps it works out.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 11/17] avformat/mxfenc: Store locally whether DNXHD profile is interlaced
tis 2021-11-09 klockan 22:48 +0100 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> > > It is just a flag per supported CID. So there is no reason to use
> > > an avpriv function for this purpose.
> >
> > This is data duplication. Honestly these ULs should probably live
> > in
> > dnxhddata.c.
> >
>
> But aren't these ULs mxf-specific? So the right place for them is
> here
> in libavformat.
> And the amount of data duplicated is trivial; furthermore adding a
> new
> DNXHD profile then and now requires modifications in two tables, so I
> don't see a maintenance burden either.
Right, this improves lavc/lavf separation. I suppose that's OK. Still
not the biggest fan of having this kind of data in more than one place,
but on the other hand it's constants..
> > >
> > > Signed-off-by: Andreas Rheinhardt
> > >
> > > ---
> > > libavformat/mxfenc.c | 47 ++
> > >
> > > --
> > > 1 file changed, 23 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > index aa9857fcff..326ec6a7d6 100644
> > > --- a/libavformat/mxfenc.c
> > > +++ b/libavformat/mxfenc.c
> > > @@ -2036,29 +2036,30 @@ static int
> > > mxf_parse_prores_frame(AVFormatContext *s, AVStream *st, AVPacket
> > > *pk
> > > }
> > >
> > > static const struct {
> > > - int cid;
> > > + uint16_t cid;
> > > + uint8_t interlaced;
> > > UID codec_ul;
> > > } mxf_dnxhd_codec_uls[] = {
> >
> > Not sure if the narrowing of types here does any good. You might
> > need
> > to put cid and interlaced after codec_ul. On the other hand UID is
> > uint8_t[] so perhaps it works out.
> >
> The narrowing is irrelevant, as all cid values in use fit into an
> uint16_t.
> Why would I need to put it at the end? Do you worry about padding
> between interlaced and codec_ul? In this case it doesn't matter: It
> would just be a matter of whether the padding is in the middle or at
> the
> end of the structure.
I don't actually care. I just thought it was curious given patch 04
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 01/17] avformat/mxfenc: Auto-insert h264_mp4toannexb BSF if needed
tis 2021-11-09 klockan 22:07 +0100 skrev Andreas Rheinhardt: > Tomas Härdin: > > tis 2021-11-09 klockan 18:34 +0100 skrev Andreas Rheinhardt: > > > The mxf and mxf_opatom muxer expect H.264 in Annex B format. > > > > > > Signed-off-by: Andreas Rheinhardt > > > > > > --- > > > The check here is taken from mpegtsenc. > > > > You didn't think to make both muxers share code instead of copy- > > pasting? > > > > Well, I can share it. The problem is just that I didn't really know > where it belongs: mux.c? utils.c? A new file? A new file probably, say libavformat/annexb.c Do we ever need to be able to do this kind of stuff in lavc? In that case it could maybe live there. But that again increases coupling between the two libs.. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 13/17] avformat/mxfenc: Remove redundant DNXHD frame size checks
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt:
> The actual frame_size is no longer used since commit
> 3d38e45eb85c7a2420cb48a9cd45625c28644b2e; and the check for
> "< 0" is equivalent to the CID being valid. But this is already
> ensured by mxf_dnxhd_codec_uls containing this CID.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mxfenc.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 326ec6a7d6..83f9a778fe 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2066,7 +2066,7 @@ static int
> mxf_parse_dnxhd_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt
> {
> MXFContext *mxf = s->priv_data;
> MXFStreamContext *sc = st->priv_data;
> - int i, cid, frame_size = 0;
> + int i, cid;
>
> if (mxf->header_written)
> return 1;
> @@ -2094,12 +2094,6 @@ static int
> mxf_parse_dnxhd_frame(AVFormatContext *s, AVStream *st, AVPacket *pkt
> if (!sc->component_depth)
> return 0;
>
> - if ((frame_size = avpriv_dnxhd_get_frame_size(cid)) ==
> DNXHD_VARIABLE) {
> - frame_size = avpriv_dnxhd_get_hr_frame_size(cid, st-
> >codecpar->width, st->codecpar->height);
> - }
> - if (frame_size < 0)
> - return 0;
Looks simple enough. Might want Baptiste to chime in given that he
authored 3d38e45.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 03/17] fate/mxf: Add tests for H.264 remuxing
tis 2021-11-09 klockan 19:01 +0100 skrev Andreas Rheinhardt: > These tests exhibit two bugs: Instead of using the in-band extradata > the demuxer makes up some extradata designed for AVC intra tracks > that lack in-band extradata; these files are nevertheless decodable > because of the in-band extradata. Furthermore, the frame reordering > is lost. > > Signed-off-by: Andreas Rheinhardt > --- > tests/fate/mxf.mak | 22 ++- > tests/ref/fate/mxf-remux-h264 | 37 ++ > tests/ref/fate/mxf-remux-xavc | 71 > +++ > 3 files changed, 128 insertions(+), 2 deletions(-) > create mode 100644 tests/ref/fate/mxf-remux-h264 > create mode 100644 tests/ref/fate/mxf-remux-xavc > > diff --git a/tests/fate/mxf.mak b/tests/fate/mxf.mak > index f96f4a429b..58a697cd86 100644 > --- a/tests/fate/mxf.mak > +++ b/tests/fate/mxf.mak > @@ -42,6 +42,21 @@ FATE_MXF_REMUX_PROBE-$(call ALLYES, PRORES_DECODER > MXF_MUXER) \ > += fate-mxf-remux-applehdr10 > fate-mxf-remux-applehdr10: CMD = transcode mxf > $(TARGET_SAMPLES)/mxf/Meridian-Apple_ProResProxy-HDR10.mxf mxf "-map > 0 -c copy" "-c copy -t 0.3" "" "-show_entries > format_tags:stream_side_data_list:stream=index,codec_name,codec_tag:s > tream_tags" > > +# Tests muxing H.264, in particular automatic insertion of > h264_mp4toannexb. > +# FIXME: The timestamps of the demuxed file are not properly > reordered. > +# Furthermore the extradata is wrong: It is one of the AVC intra > SPS/PPS; > +# decoding only works due to in-band extradata. Is this a problem with the samples or the code? Or both? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: support MCA audio information
> + if(channel_ordering_ptr->service_type !=
> AV_AUDIO_SERVICE_TYPE_NB) {
> + ast = (enum
> AVAudioServiceType*)av_stream_new_side_data(st,
> AV_PKT_DATA_AUDIO_SERVICE_TYPE, sizeof(*ast));
ast == NULL still needs handling here
I don't see anything else that needs fixing, except maybe splitting the
actual reordering into a separate patch as (IIRC) Marton wants
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add wpsnr video filter
fre 2021-10-29 klockan 19:43 +0200 skrev Paul B Mahol:
> On Fri, Oct 29, 2021 at 7:26 PM Tomas Härdin
> wrote:
>
> > fre 2021-10-29 klockan 19:17 +0200 skrev Paul B Mahol:
> > > On Fri, Oct 29, 2021 at 6:59 PM Tomas Härdin
> > > wrote:
> > >
> > > > fre 2021-10-29 klockan 17:00 +0200 skrev Paul B Mahol:
> > > > > On Fri, Oct 29, 2021 at 4:46 PM Tomas Härdin <
> > > > > tjop...@acc.umu.se>
> > > > > wrote:
> > > > >
> > > > > > fre 2021-10-29 klockan 15:33 +0200 skrev Paul B Mahol:
> > > > > > > On Fri, Oct 29, 2021 at 3:12 PM Tomas Härdin <
> > > > > > > tjop...@acc.umu.se>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > +static double get_hx(const uint8_t *src, int
> > > > > > > > > linesize,
> > > > > > > > > int
> > > > > > > > > w,
> > > > > > > > > int h)
> > > > > > > > > +{
> > > > > > > > > + int64_t sum = 0;
> > > > > > > > > +
> > > > > > > > > + for (int y = 0; y < h; y++) {
> > > > > > > > > + for (int x = 0; x < w; x++) {
> > > > > > > > > + sum += 12 * src[x] -
> > > > > > > > > + 2 * (src[x-1] + src[x+1] +
> > > > > > > > > + src[x + linesize] +
> > > > > > > > > + src[x - linesize]) -
> > > > > > > > > + 1 * (src[x - 1 - linesize] +
> > > > > > > > > + src[x + 1 - linesize] +
> > > > > > > > > + src[x - 1 + linesize] +
> > > > > > > > > + src[x + 1 + linesize]);
> > > > > > > > > + }
> > > > > > > > > +
> > > > > > > > > + src += linesize;
> > > > > > > > > + }
> > > > > > > > > +
> > > > > > > > > + return fabs(sum * 0.25);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static double get_hx16(const uint8_t *ssrc, int
> > > > > > > > > linesize,
> > > > > > > > > int w,
> > > > > > > > > int
> > > > > > > > > h)
> > > > > > > > > +{
> > > > > > > > > + const uint16_t *src = (const uint16_t *)ssrc;
> > > > > > > >
> > > > > > > > This is not -fstrict-aliasing safe
> > > > > > > >
> > > > > > >
> > > > > > > How so? I get no warnings at all, and similar is used
> > > > > > > everywhere
> > > > > > > else.
> > > > > >
> > > > > > Then those places should be fixed. We have macros like
> > > > > > AV_RB16()
> > > > > > for a
> > > > > > reason. That gcc doesn't warn about this doesn't mean it
> > > > > > isn't
> > > > > > free
> > > > > > to
> > > > > > assume ssrc and src points to different non-overlapping
> > > > > > regions
> > > > > > of
> > > > > > memory.
> > > > > >
> > > > > >
> > > > > That is sub optimal and unacceptable solution.
> > > >
> > > > Did you do measurements to come to this conclusion?
> > > >
> > > > > Review remark ignored.
> > > >
> > > > Undefined behavior is *not* acceptable. If you want this file
> > > > specifically to be compiled with strict aliasing disabled then
> > > > you
> > > > must
> > > > at the very least change the build system accordingly
> > > >
> > >
> > > Cite source that can prove this.
> >
> > The gcc manpage
> >
>
> citations needed.
>
>
> >
> > > Compilers are silent about this.
> >
> > Irrelevant
> >
>
>
> citations needed.
>
>
> >
> > > There are hundredths if not thousands of such examples across
> > > codebase.
> >
> > This is not a valid excuse. We need less UB in the codebase, not
> > more.
> > UBs beget CVEs.
> >
>
> citations needed.
Childish behavior doesn't make you right, Paul.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4] avformat/mxfdec: Check component_depth in mxf_get_color_range()
lör 2021-12-04 klockan 22:32 +0100 skrev Michael Niedermayer: > Fixes: shift exponent 4294967163 is too large for 32-bit type 'int' > Fixes: 41449/clusterfuzz-testcase-minimized-ffmpeg_IO_DEMUXER_fuzzer- > 6183636217495552 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavformat/mxfdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > index af9d33f7969..c231c944c01 100644 > --- a/libavformat/mxfdec.c > +++ b/libavformat/mxfdec.c > @@ -2274,12 +2274,12 @@ static enum AVColorRange > mxf_get_color_range(MXFContext *mxf, MXFDescriptor *des > /* CDCI range metadata */ > if (!descriptor->component_depth) > return AVCOL_RANGE_UNSPECIFIED; > - if (descriptor->black_ref_level == 0 && > + if (descriptor->black_ref_level == 0 && descriptor- > >component_depth < 31 && > descriptor->white_ref_level == ((1< >component_depth) - 1) && > (descriptor->color_range == (1< >component_depth) || > descriptor->color_range == ((1< >component_depth) - 1))) > return AVCOL_RANGE_JPEG; > - if (descriptor->component_depth >= 8 && > + if (descriptor->component_depth >= 8 && descriptor- > >component_depth < 31 && > descriptor->black_ref_level == (1 <<(descriptor- > >component_depth - 4)) && > descriptor->white_ref_level == (235<<(descriptor- > >component_depth - 8)) && > descriptor->color_range == ((14<<(descriptor- > >component_depth - 4)) + 1)) Looks OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 5/7] avformat/mxfdec: Check for duplicate mxf_read_index_entry_array()
sön 2021-12-05 klockan 22:19 +0100 skrev Michael Niedermayer:
> Fixes: memleak
> Fixes: 41596/clusterfuzz-testcase-minimized-ffmpeg_dem_MXF_fuzzer-
> 6439060204290048
>
> Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/mxfdec.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index c231c944c01..1d501982793 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -,6 +,9 @@ static int
> mxf_read_index_entry_array(AVIOContext *pb, MXFIndexTableSegment *seg
> {
> int i, length;
>
> + if (segment->temporal_offset_entries)
> + return AVERROR_INVALIDDATA;
> +
> segment->nb_index_entries = avio_rb32(pb);
>
> length = avio_rb32(pb);
Should be OK. Not sure if the spec allows multiple IndexEntryArrays per
index table, but this at least shouldn't break anything since it
wouldn't have been working correctly before either way.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs
fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:
> > Please add a reference to the relevant SMPTE document in the
> > comment, or perhaps at the list of references at the start of the
> > file
> >
> > /Tomas
>
> I have added the reference to ST2019-4 for "VC3 mapping", so that
> should be ok for generic standard files.
> It seems redundant for me, but if you want, I could add the link to
> the online register where the container ul is public ?
> https://registry.smpte-ra.org/apps/pages/
>
> Concerning the essence key, it is more tricky because of AVID in the
> place...
> To start with, apart AVID, all frame-wrapped samples I have (I can
> share them with you but not all of them publicly), do respect the
> standard
> - frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always 0x0C
> ("DNxHD" frame-mapping)
> There are up to date publicly available ARRI samples where 0x0C is
> used here:
> https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage
>
> But I also have an AVID Op1a file where the value 0x05 is used
> ("MPEG" frame-mapping, ie. s381m).
> And concerning OPAtom, Philip de Nier has an AVID sample where the
> value 0x06 is used ("MPEG" clip-wrapping).
>
> So, what is apparent at the end is that :
> - apart from AVID, the standard values 0x0c/0x0d are used
> - AVID uses the values from the older "MPEG mapping" (ie smpte 381m)
>
> Now :
> - currently ffmpeg uses 0x05 for OPatom which does not follow any
> implementation and seems bad
> - it seems there is a consensus (incl. AVID) to always use 0x05 or
> 0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping (OPAtom)
> => follow either s381m or st2019-4
> - it seems clear ffmpeg shall take the "standard-flavor" for generic
> OP's, so 0x0C for frame-based wrapping
> - it is less clear about OPAtom which is rather an AVID-hack-thing,
> but it should be moved to either 0x06 or 0x0d
> - I have discussed this with philip de nier, and bmx (a reference
> software in my opinion) will stick to the AVID form, so 0x06. And I
> think it is reasonable, since OPAtom/Avid are almost the same damn
> thing
>
> Note: no matter the essence key, the link between the tracks and the
> body with the TrackNumber always work, so it seems there are not much
> interoperability issues with it.
Seems I missed the reference to the VC-3 spec somehow, sorry. Since it
is Matthieu Bouron who added this initially, you should really talk to
him. I care less about mxfenc than I do mxfdec, since the latter can
more easily induce CVEs.
That said, if we want this to work correctly for everyone then we need
a proper set of tests. We also need to go over all the specs, and what
all implementations are currently doing. Which is a lot of work. Or a
lot of billable hours!
This is why I've said for years now that we should delete mxfenc and
just bring in bmx. We don't need what little free software people there
are who know MXF to keep maintaining two codebases.
We could just bring in bmx as a subtree and be done with it. Delete all
MXF code native to FFmpeg, put all effort into bmx. People in this
project suffer from the belief that code is valuable rather than a
liability. Worse is not better. Correct is better.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: fix DNxHD GC ULs
ons 2021-12-08 klockan 02:18 +0100 skrev Marton Balint:
>
>
> On Wed, 8 Dec 2021, Tomas Härdin wrote:
>
> > fre 2021-12-03 klockan 09:38 + skrev Nicolas Gaullier:
> > > > Please add a reference to the relevant SMPTE document in the
> > > > comment, or perhaps at the list of references at the start of
> > > > the
> > > > file
> > > >
> > > > /Tomas
> > >
> > > I have added the reference to ST2019-4 for "VC3 mapping", so that
> > > should be ok for generic standard files.
> > > It seems redundant for me, but if you want, I could add the link
> > > to
> > > the online register where the container ul is public ?
> > > https://registry.smpte-ra.org/apps/pages/
> > >
> > > Concerning the essence key, it is more tricky because of AVID in
> > > the
> > > place...
> > > To start with, apart AVID, all frame-wrapped samples I have (I
> > > can
> > > share them with you but not all of them publicly), do respect the
> > > standard
> > > - frame-wrapped : ARRI, Adobe Media Encoder, Harmonic : always
> > > 0x0C
> > > ("DNxHD" frame-mapping)
> > > There are up to date publicly available ARRI samples where 0x0C
> > > is
> > > used here:
> > >
> > > https://www.arri.com/en/learn-help/learn-help-camera-system/camera-sample-footage
> > >
> > > But I also have an AVID Op1a file where the value 0x05 is used
> > > ("MPEG" frame-mapping, ie. s381m).
> > > And concerning OPAtom, Philip de Nier has an AVID sample where
> > > the
> > > value 0x06 is used ("MPEG" clip-wrapping).
> > >
> > > So, what is apparent at the end is that :
> > > - apart from AVID, the standard values 0x0c/0x0d are used
> > > - AVID uses the values from the older "MPEG mapping" (ie smpte
> > > 381m)
> > >
> > > Now :
> > > - currently ffmpeg uses 0x05 for OPatom which does not follow any
> > > implementation and seems bad
> > > - it seems there is a consensus (incl. AVID) to always use 0x05
> > > or
> > > 0x0C for frame-wrapping and 0x06 or 0x0d for clip-wrapping
> > > (OPAtom)
> > > => follow either s381m or st2019-4
> > > - it seems clear ffmpeg shall take the "standard-flavor" for
> > > generic
> > > OP's, so 0x0C for frame-based wrapping
> > > - it is less clear about OPAtom which is rather an AVID-hack-
> > > thing,
> > > but it should be moved to either 0x06 or 0x0d
> > > - I have discussed this with philip de nier, and bmx (a reference
> > > software in my opinion) will stick to the AVID form, so 0x06. And
> > > I
> > > think it is reasonable, since OPAtom/Avid are almost the same
> > > damn
> > > thing
> > >
> > > Note: no matter the essence key, the link between the tracks and
> > > the
> > > body with the TrackNumber always work, so it seems there are not
> > > much
> > > interoperability issues with it.
> >
> > Seems I missed the reference to the VC-3 spec somehow, sorry. Since
> > it
> > is Matthieu Bouron who added this initially, you should really talk
> > to
> > him. I care less about mxfenc than I do mxfdec, since the latter
> > can
> > more easily induce CVEs.
> >
> > That said, if we want this to work correctly for everyone then we
> > need
> > a proper set of tests. We also need to go over all the specs, and
> > what
> > all implementations are currently doing. Which is a lot of work. Or
> > a
> > lot of billable hours!
>
> Nicolas already made a reasonable investigation, so I am not sure
> what
> else is required. FWIW there is also a trac ticket with this issue:
>
> https://trac.ffmpeg.org/ticket/6380
I suppose we should just try to get a hold of Matthieu then, see if it
breaks any of his stuff
> >
> > This is why I've said for years now that we should delete mxfenc
> > and
> > just bring in bmx. We don't need what little free software people
> > there
> > are who know MXF to keep maintaining two codebases.
> >
> > We could just bring in bmx as a subtree and be done with it. Delete
> > all
> > MXF code native to FFmpeg, put all effort into bmx. People in this
> > project suffer from the belief that code is valuable rather than a
> > liability. Worse is not better. Correct is better.
>
> Tested and polished code is valuable, and I thin
Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxfenc: fix DNxHD GC container_ul
tis 2021-12-14 klockan 16:27 +0100 skrev Nicolas Gaullier:
> Signed-off-by: Nicolas Gaullier
> ---
> libavformat/mxfenc.c | 2 +-
> tests/ref/lavf/mxf_opatom | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index fcd9afda2a..512baa45d3 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -181,7 +181,7 @@ static const MXFContainerEssenceEntry
> mxf_essence_container_uls[] = {
> {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x00
> ,0x00,0x00 },
> mxf_write_cdci_desc },
> // DNxHD
> - { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x11
> ,0x01,0x00 },
> + { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11
> ,0x01,0x00 },
> {
> 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01
> ,0x05,0x00 },
> {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01
> ,0x00,0x00 },
> mxf_write_cdci_desc },
> diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
> index 61e70b..b818f26c36 100644
> --- a/tests/ref/lavf/mxf_opatom
> +++ b/tests/ref/lavf/mxf_opatom
> @@ -1,3 +1,3 @@
> -5d235c127ace64b1f4fe6c79a7ca8be6 *tests/data/lavf/lavf.mxf_opatom
> +e558e50a94d88762e07ab8149aced7b9 *tests/data/lavf/lavf.mxf_opatom
> 4717625 tests/data/lavf/lavf.mxf_opatom
> tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a
Should be fine based on previous discussion
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: fix DNxHD GC element_type
tis 2021-12-14 klockan 16:27 +0100 skrev Nicolas Gaullier:
> The values for the essence element type were updated in the spec
> from 0x05/0x06 (ST2019-4 2008) to 0x0C/0x0D (ST2019-4 2009).
>
> Fixes ticket #6380.
>
> Thanks-to: Philip de Nier
> Thanks-to: Matthieu Bouron
>
> Signed-off-by: Nicolas Gaullier
> ---
> libavformat/mxfenc.c | 9 -
> tests/ref/lavf/mxf_opatom | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 512baa45d3..bae41a774d 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -32,6 +32,7 @@
> * SMPTE 379M MXF Generic Container
> * SMPTE 381M Mapping MPEG Streams into the MXF Generic Container
> * SMPTE 422M Mapping JPEG 2000 Codestreams into the MXF Generic
> Container
> + * SMPTE ST2019-4 (2009 or later) Mapping VC-3 Coding Units into the
> MXF Generic Container
> * SMPTE RP210: SMPTE Metadata Dictionary
> * SMPTE RP224: Registry of SMPTE Universal Labels
> */
> @@ -182,7 +183,7 @@ static const MXFContainerEssenceEntry
> mxf_essence_container_uls[] = {
> mxf_write_cdci_desc },
> // DNxHD
> { {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x0D,0x01,0x03,0x01,0x02,0x11
> ,0x01,0x00 },
> - {
> 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01
> ,0x05,0x00 },
> + {
> 0x06,0x0E,0x2B,0x34,0x01,0x02,0x01,0x01,0x0D,0x01,0x03,0x01,0x15,0x01
> ,0x0C,0x00 },
> {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x0A,0x04,0x01,0x02,0x02,0x71,0x01
> ,0x00,0x00 },
> mxf_write_cdci_desc },
> // JPEG2000
> @@ -2674,6 +2675,12 @@ static int mxf_init(AVFormatContext *s)
>
> memcpy(sc->track_essence_element_key,
> mxf_essence_container_uls[sc->index].element_ul, 15);
> sc->track_essence_element_key[15] = present[sc->index];
> + if (s->oformat == _mxf_opatom_muxer && st->codecpar-
> >codec_id == AV_CODEC_ID_DNXHD) {
> + // clip-wrapping requires 0x0D per ST2019-4:2009 or 0x06
> per previous version ST2019-4:2008
> + // we choose to use 0x06 instead 0x0D to be compatible
> with AVID systems
> + // and produce mxf files with the most relevant flavour
> for opatom
> + sc->track_essence_element_key[14] = 0x06;
> + }
> PRINT_KEY(s, "track essence element key", sc-
> >track_essence_element_key);
>
> if (!present[sc->index])
> diff --git a/tests/ref/lavf/mxf_opatom b/tests/ref/lavf/mxf_opatom
> index b818f26c36..e34cf2559e 100644
> --- a/tests/ref/lavf/mxf_opatom
> +++ b/tests/ref/lavf/mxf_opatom
> @@ -1,3 +1,3 @@
> -e558e50a94d88762e07ab8149aced7b9 *tests/data/lavf/lavf.mxf_opatom
> +aab6397829bd90f0c77a3f9fde53bb9c *tests/data/lavf/lavf.mxf_opatom
> 4717625 tests/data/lavf/lavf.mxf_opatom
> tests/data/lavf/lavf.mxf_opatom CRC=0xf55aa22a
Should be fine based on previous discussion, and because Matthieu is
fine with it, being the one who added DNxHD support initially
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] request to revert all patches that made cfr decoders vfr
fre 2021-12-10 klockan 08:54 +0100 skrev Paul B Mahol: > This is the first warning to revert all patches that made cfr > decoders vfr. Would be good if you named said patches /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6 1/2] libavcodec: Added DFPWM1a codec
tor 2022-03-03 klockan 10:44 -0500 skrev Jack Bruienne: > > From the wiki page (https://wiki.vexatos.com/dfpwm): > > DFPWM (Dynamic Filter Pulse Width Modulation) is an audio codec > > created by Ben “GreaseMonkey” Russell in 2012, originally to be > > used > > as a voice codec for asiekierka's pixmess, a C remake of 64pixels. > > It is a 1-bit-per-sample codec which uses a dynamic-strength one- > > pole > > low-pass filter as a predictor. Due to the fact that a raw DPFWM > > decoding > > creates a high-pitched whine, it is often followed by some post- > > processing > > filters to make the stream more listenable. This sounds similar to something I wrote for the Atari 2600 a number of years ago ( https://www.pouet.net/prod.php?which=59283 ) I found an encoder online for DFPWM, and it seems to suffer from the same "beeping" that my debeeping hack fixes (suppressing 0xAA and 0x55 bytes). Perhaps a similar hack could be useful in dfpwmenc.c > > It has recently gained popularity through the ComputerCraft mod for > Minecraft, which added support for audio through this codec, as well > as > the Computronics expansion which preceeded the official support. > These > both implement the slightly adjusted 1a version of the codec, which > is > the version I have chosen for this patch. > > This patch adds a new codec (with encoding and decoding) for DFPWM1a. > The codec sources are pretty simple: they use the reference codec > with > a basic wrapper to connect it to the FFmpeg AVCodec system. > > To clarify, the codec does not have a specific sample rate - it is > provided by the container (or user), which is typically 48000, but > has > also been known to be 32768. The codec does not specify channel info > either, and it's pretty much always used with one mono channel. > However, since it appears that libavcodec expects both sample rate > and > channel count to be handled by either the codec or container, I have > made the decision to allow multiple channels interleaved, which as > far > as I know has never been used, but it works fine here nevertheless. From experience it's usually better to be strict when it comes to stuff like this. The ComputerCraft people should work out a standard for this, preferably a container. We've had a similar problem in FreeDV where which codec was being used was implicit most of the time, which has been resolved with the .c2 format. > --- > Changelog | 1 + > MAINTAINERS | 1 + > doc/general_contents.texi | 1 + > libavcodec/Makefile | 2 + > libavcodec/allcodecs.c | 2 + > libavcodec/codec_desc.c | 7 ++ > libavcodec/codec_id.h | 1 + > libavcodec/dfpwmdec.c | 134 > ++ > libavcodec/dfpwmenc.c | 121 ++ > libavcodec/utils.c | 2 + > libavcodec/version.h | 4 +- > 11 files changed, 274 insertions(+), 2 deletions(-) > create mode 100644 libavcodec/dfpwmdec.c > create mode 100644 libavcodec/dfpwmenc.c Patch doesn't apply on current master (e645a1d) /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6 1/2] libavcodec: Added DFPWM1a codec
mån 2022-03-07 klockan 22:04 -0500 skrev Jack Bruienne:
> On 3/7/22 06:03, Tomas Härdin wrote:
>
> > tor 2022-03-03 klockan 10:44 -0500 skrev Jack Bruienne:
> > > From the wiki page (https://wiki.vexatos.com/dfpwm):
> > > > DFPWM (Dynamic Filter Pulse Width Modulation) is an audio codec
> > > > created by Ben “GreaseMonkey” Russell in 2012, originally to be
> > > > used
> > > > as a voice codec for asiekierka's pixmess, a C remake of
> > > > 64pixels.
> > > > It is a 1-bit-per-sample codec which uses a dynamic-strength
> > > > one-
> > > > pole
> > > > low-pass filter as a predictor. Due to the fact that a raw
> > > > DPFWM
> > > > decoding
> > > > creates a high-pitched whine, it is often followed by some
> > > > post-
> > > > processing
> > > > filters to make the stream more listenable.
> > This sounds similar to something I wrote for the Atari 2600 a
> > number of
> > years ago (https://www.pouet.net/prod.php?which=59283 )
> >
> > I found an encoder online for DFPWM, and it seems to suffer from
> > the
> > same "beeping" that my debeeping hack fixes (suppressing 0xAA and
> > 0x55
> > bytes). Perhaps a similar hack could be useful in dfpwmenc.c
>
> I'm curious how this works. Do you just cut out those bytes from the
> encoder output, or is it modified in some way? Wouldn't removing the
> data entirely eventually cause much of the audio to be lost?
The source code is included in the release. Look at audioquant.cpp.
I've attached it for convenience. The codec is based on a state machine
where each state is a 5-bit PCM value that can go to either of two
states, also 5-bit PCM values. Hence 1 bit per sample. I also have a
low-pass filter in the decoder.
I penalize state machines which result in 0x55 and 0xAA being overly
represented. This is done via computing a histogram of the output bytes
and scaling the RMS error according to how many of those bytes are in
the (tentative) output.
Another approach could be to detect and blank excessive runs of 0x55
and 0xAA bytes.
> > From experience it's usually better to be strict when it comes to
> > stuff
> > like this. The ComputerCraft people should work out a standard for
> > this, preferably a container. We've had a similar problem in FreeDV
> > where which codec was being used was implicit most of the time,
> > which
> > has been resolved with the .c2 format.
>
> I think the best standardized container for DFPWM will be WAV.
I agree, and I see this was already pushed.
/Tomas
#define _CRT_SECURE_NO_WARNINGS
#include
#include
#include
#include
#include
#include
using namespace std;
typedef short sample_t;
typedef int64_t err_t;
static vector samples;
static int rate;
#define DESIRED_RMS2//can be tweaked slightly for better utilization of dynamic range depending on material
#define BPS1
#define DELTA 10//try table values +- this every pass
//higher values = more exhaustive search
#define PRESHAPE
//#define POSTSHAPE
//#define VERBOSE
static void normalize(sample_t *data, int n) {
float rms = 0;
int x;
for (x = 0; x < n; x++)
rms += data[x]*data[x];
rms = sqrtf(rms / n);
fprintf(stderr, "RMS amplitude prior to normalization: %f\n", rms);
for (x = 0; x < n; x++) {
int value = data[x] * DESIRED_RMS / rms;
if (value < -32768) value = -32768;
if (value > 32767) value = 32767;
data[x] = value;
}
}
static void quantize(const sample_t *input, sample_t *output, int n) {
int x, y, last_error = 0;
int counts[31][31];
memset(counts, 0, sizeof(counts));
for (x = 0; x < n; x++) {
#ifdef PRESHAPE
//shape using feedback. not sure how correct this is,
//but quiet parts appear to receive less noise
output[x] = ((input[x] + 3 * last_error / 4) / 2048) + 15;
#else
output[x] = (input[x] / 2048) + 15;
#endif
if (output[x] < 0)
output[x] = 0;
if (output[x] >= 31)
output[x] = 30;
last_error = (output[x] - 15) * 2048 - input[x];
if (x > 0)
counts[output[x-1]][output[x]]++;
}
for (y = 0; y < 31; y++) {
for (x = 0; x < 31; x++)
fprintf(stderr, "%3i ", counts[y][x]);
fprintf(stderr, "\n");
}
}
//table is k*31 entries, where k=2^N
static err_t table_adpcm_work(sample_t *data, int n, int *table, int k, int do_output, unsigned char *bits) {
int x, last = 15, last_error = 0;
err_t ret = 0;
int hist[256] = {0};
float factor;
int byte = 0;
for (x = 0; x < n; x++) {
i
Re: [FFmpeg-devel] [PATCH v7 2/3] libavformat: Add DFPWM raw format
+static const AVOption dfpwm_options[] = {
+{ "sample_rate", "", offsetof(DFPWMAudioDemuxerContext,
sample_rate), AV_OPT_TYPE_INT, {.i64 = 48000}, 0, INT_MAX,
AV_OPT_FLAG_DECODING_PARAM },
+{ "channels","", offsetof(DFPWMAudioDemuxerContext, channels),
AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM },
+{ NULL },
+};
I think you can set the input sample rate and channel count via the
FFmpeg CLI's -ar and -ac options and have them be carried in and out of
here
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 2/3] libavformat: Add DFPWM raw format
mån 2022-03-14 klockan 10:56 +0100 skrev Paul B Mahol:
> On Mon, Mar 14, 2022 at 10:47 AM Tomas Härdin
> wrote:
>
> > +static const AVOption dfpwm_options[] = {
> > + { "sample_rate", "", offsetof(DFPWMAudioDemuxerContext,
> > sample_rate), AV_OPT_TYPE_INT, {.i64 = 48000}, 0, INT_MAX,
> > AV_OPT_FLAG_DECODING_PARAM },
> > + { "channels", "", offsetof(DFPWMAudioDemuxerContext,
> > channels),
> > AV_OPT_TYPE_INT, {.i64 = 1}, 0, INT_MAX, AV_OPT_FLAG_DECODING_PARAM
> > },
> > + { NULL },
> > +};
> >
> > I think you can set the input sample rate and channel count via the
> > FFmpeg CLI's -ar and -ac options and have them be carried in and
> > out of
> > here
> >
>
> I highly doubt that.
You can for other raw formats:
ffplay -f s16le -ar 8k -ac 1 /dev/urandom
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 03/21] avcodec/zmbv: Use ff_inflate_init/end()
tis 2022-03-15 klockan 21:05 +0100 skrev Andreas Rheinhardt: > Returns better error messages in case of error and deduplicates > the inflateInit() code. > > Signed-off-by: Andreas Rheinhardt > --- > configure | 2 +- > libavcodec/zmbv.c | 38 ++ > 2 files changed, 15 insertions(+), 25 deletions(-) Looks OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 02/21] avcodec/zlib_wrapper: Add wrappers for zlib inflateInit, inflateEnd
ons 2022-03-16 klockan 20:32 +0100 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > > +int ff_inflate_init(FFZStream *z, void *logctx)
> > > +{
> > > + z_stream *const zstream = >zstream;
> > > + int zret;
> > > +
> > > + z->inited = 0;
> > > + zstream->next_in = Z_NULL;
> > > + zstream->avail_in = 0;
> > > + zstream->zalloc = Z_NULL;
> > > + zstream->zfree = Z_NULL;
> > > + zstream->opaque = Z_NULL;
> >
> > why not bzero()?
> >
> > Rest of the patch looks fine
> >
>
> bzero()? You mean memset to zero? The reason that I initialize
> exactly
> these fields is because these are exactly the fields required to be
> initialized by zlib (for inflate; next_in and avail_in are not
> required
> to be initialized for deflate): "The fields next_in, avail_in,
> zalloc,
> zfree and opaque must be initialized before by the caller." zlib
> treats
> all the other fields as uninitialized, so why should we initialize
> them
> (Actually reinitialize them -- most FFZStreams are already zeroed
> initially as part of a codec's private context.)? The way it is done
> in
> this patch shows directly which elements zlib expects to be set;
> setting
> everything would not achieve the same.
Right. Looks OK then
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID
ons 2022-03-16 klockan 20:38 +0100 skrev Marton Balint: > > > On Wed, 16 Mar 2022, Tomas Härdin wrote: > > > mån 2022-03-14 klockan 21:44 +0100 skrev Marton Balint: > > > > > > > > > On Mon, 14 Mar 2022, Tomas Härdin wrote: > > > > > > > mån 2022-03-14 klockan 20:54 +0100 skrev Marton Balint: > > > > > > > > > > > > > > > On Mon, 14 Mar 2022, Tomas Härdin wrote: > > > > > > > > > > > mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint: > > > > > > > Only index tables repeating previous index tables should > > > > > > > use > > > > > > > the > > > > > > > same > > > > > > > InstaceUID. Use the index start position when generating > > > > > > > the > > > > > > > InstanceUID to fix > > > > > > > this. > > > > > > > > > > > > > > Signed-off-by: Marton Balint > > > > > > > --- > > > > > > > libavformat/mxfenc.c | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > > > > > > > index ba8e7babfb..5b972eadaa 100644 > > > > > > > --- a/libavformat/mxfenc.c > > > > > > > +++ b/libavformat/mxfenc.c > > > > > > > @@ -1757,7 +1757,7 @@ static void > > > > > > > mxf_write_index_table_segment(AVFormatContext *s) > > > > > > > > > > > > > > // instance id > > > > > > > mxf_write_local_tag(s, 16, 0x3C0A); > > > > > > > - mxf_write_uuid(pb, IndexTableSegment, 0); > > > > > > > + mxf_write_uuid(pb, IndexTableSegment, mxf- > > > > > > > > last_indexed_edit_unit); > > > > > > > > > > > > Two things: yes, it is good that this fixes the same > > > > > > InstanceUID > > > > > > being > > > > > > reused. But more importantly, we should not be writing > > > > > > files > > > > > > with > > > > > > over > > > > > > 65536 partitions! > > > > > > > > > > last_indexed_edit_unit is frame based not partition based, so > > > > > it > > > > > can > > > > > overflow 65536 realtively easily, that is why I submitted > > > > > patch > > > > > 1. > > > > > > > > Right. But we could use the partition number instead. > > > > > > Well, we could use mxf->body_partitions_count but it is not > > > trivial > > > to see > > > that it will work for all cases. > > > > I don't see why not. But upping to 32-bit is easy anyways. > > I tried, but body partition count is the same for the last body > partition > and for the footer partition, both having different index tables... > > So I still find it more starightforward to use index start position > instead of some magic to find out the proper partition count, is it > fine > with you? I mean it shouldn't matter so long as its unique. So sure /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 18/21] avcodec/zmbvenc: Use ff_deflate_init/end() wrappers
tis 2022-03-15 klockan 21:06 +0100 skrev Andreas Rheinhardt: > They emit better error messages (it does not claim that inflateInit > failed upon an error from deflateInit!) and uses our allocation > functions. > > Signed-off-by: Andreas Rheinhardt > --- > configure | 2 +- > libavcodec/zmbvenc.c | 41 +++-- > 2 files changed, 16 insertions(+), 27 deletions(-) Looks OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 02/21] avcodec/zlib_wrapper: Add wrappers for zlib inflateInit, inflateEnd
> +int ff_inflate_init(FFZStream *z, void *logctx)
> +{
> + z_stream *const zstream = >zstream;
> + int zret;
> +
> + z->inited = 0;
> + zstream->next_in = Z_NULL;
> + zstream->avail_in = 0;
> + zstream->zalloc = Z_NULL;
> + zstream->zfree = Z_NULL;
> + zstream->opaque = Z_NULL;
why not bzero()?
Rest of the patch looks fine
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID
mån 2022-03-14 klockan 21:44 +0100 skrev Marton Balint: > > > On Mon, 14 Mar 2022, Tomas Härdin wrote: > > > mån 2022-03-14 klockan 20:54 +0100 skrev Marton Balint: > > > > > > > > > On Mon, 14 Mar 2022, Tomas Härdin wrote: > > > > > > > mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint: > > > > > Only index tables repeating previous index tables should use > > > > > the > > > > > same > > > > > InstaceUID. Use the index start position when generating the > > > > > InstanceUID to fix > > > > > this. > > > > > > > > > > Signed-off-by: Marton Balint > > > > > --- > > > > > libavformat/mxfenc.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > > > > > index ba8e7babfb..5b972eadaa 100644 > > > > > --- a/libavformat/mxfenc.c > > > > > +++ b/libavformat/mxfenc.c > > > > > @@ -1757,7 +1757,7 @@ static void > > > > > mxf_write_index_table_segment(AVFormatContext *s) > > > > > > > > > > // instance id > > > > > mxf_write_local_tag(s, 16, 0x3C0A); > > > > > - mxf_write_uuid(pb, IndexTableSegment, 0); > > > > > + mxf_write_uuid(pb, IndexTableSegment, mxf- > > > > > > last_indexed_edit_unit); > > > > > > > > Two things: yes, it is good that this fixes the same > > > > InstanceUID > > > > being > > > > reused. But more importantly, we should not be writing files > > > > with > > > > over > > > > 65536 partitions! > > > > > > last_indexed_edit_unit is frame based not partition based, so it > > > can > > > overflow 65536 realtively easily, that is why I submitted patch > > > 1. > > > > Right. But we could use the partition number instead. > > Well, we could use mxf->body_partitions_count but it is not trivial > to see > that it will work for all cases. I don't see why not. But upping to 32-bit is easy anyways. > For simple indexes, we rewrite the index > table in the footer when writing the mxf header, opatom may follow > another > layout, so it just felt less error-prone to use actually the start > offset > of the index. We only need to do this for frame wrapping. And yeah that can be a separate patch. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID
mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint: > Only index tables repeating previous index tables should use the same > InstaceUID. Use the index start position when generating the > InstanceUID to fix > this. > > Signed-off-by: Marton Balint > --- > libavformat/mxfenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > index ba8e7babfb..5b972eadaa 100644 > --- a/libavformat/mxfenc.c > +++ b/libavformat/mxfenc.c > @@ -1757,7 +1757,7 @@ static void > mxf_write_index_table_segment(AVFormatContext *s) > > // instance id > mxf_write_local_tag(s, 16, 0x3C0A); > - mxf_write_uuid(pb, IndexTableSegment, 0); > + mxf_write_uuid(pb, IndexTableSegment, mxf- > >last_indexed_edit_unit); Two things: yes, it is good that this fixes the same InstanceUID being reused. But more importantly, we should not be writing files with over 65536 partitions! This has been bugging me for quite some time. Honestly I don't know why the decision was taken initially to write indices every 10 seconds. In any use-case where seeks are moderately expensive working with files produced by mxfenc is a nightmare. Prime example being HTTP. If we do still need to keep writing partitions this way, can we repeat the IndexTableSegments in the footer so the entire file doesn't have to be scanned? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/4] avformat/mxfdec: Check for avio_read() failure in mxf_read_strong_ref_array()
sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/mxfdec.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index d7cdd22c8a..828fc0f9f1 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -932,6 +932,7 @@ static int mxf_read_cryptographic_context(void
> *arg, AVIOContext *pb, int tag, i
>
> static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> int *count)
> {
> + int64_t ret;
> unsigned c = avio_rb32(pb);
>
> //avio_read() used int
> @@ -946,7 +947,12 @@ static int mxf_read_strong_ref_array(AVIOContext
> *pb, UID **refs, int *count)
> return AVERROR(ENOMEM);
> }
> avio_skip(pb, 4); /* useless size of objects, always 16
> according to specs */
> - avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> + ret = avio_read(pb, (uint8_t *)*refs, *count * sizeof(UID));
> + if (ret != *count * sizeof(UID)) {
> + *count = ret < 0 ? 0 : ret / sizeof(UID);
> + return ret < 0 ? ret : AVERROR_INVALIDDATA;
Looks ok
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] avformat/mxfenc: do not write index tables with the same InstanceUID
mån 2022-03-14 klockan 20:54 +0100 skrev Marton Balint: > > > On Mon, 14 Mar 2022, Tomas Härdin wrote: > > > mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint: > > > Only index tables repeating previous index tables should use the > > > same > > > InstaceUID. Use the index start position when generating the > > > InstanceUID to fix > > > this. > > > > > > Signed-off-by: Marton Balint > > > --- > > > libavformat/mxfenc.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c > > > index ba8e7babfb..5b972eadaa 100644 > > > --- a/libavformat/mxfenc.c > > > +++ b/libavformat/mxfenc.c > > > @@ -1757,7 +1757,7 @@ static void > > > mxf_write_index_table_segment(AVFormatContext *s) > > > > > > // instance id > > > mxf_write_local_tag(s, 16, 0x3C0A); > > > - mxf_write_uuid(pb, IndexTableSegment, 0); > > > + mxf_write_uuid(pb, IndexTableSegment, mxf- > > > > last_indexed_edit_unit); > > > > Two things: yes, it is good that this fixes the same InstanceUID > > being > > reused. But more importantly, we should not be writing files with > > over > > 65536 partitions! > > last_indexed_edit_unit is frame based not partition based, so it can > overflow 65536 realtively easily, that is why I submitted patch 1. Right. But we could use the partition number instead. > > > > > This has been bugging me for quite some time. Honestly I don't know > > why > > the decision was taken initially to write indices every 10 seconds. > > In > > any use-case where seeks are moderately expensive working with > > files > > produced by mxfenc is a nightmare. Prime example being HTTP. > > The 10 second body partition limit is coming from some specification > (XDCAM HD?), so this is kind of intentional. > > > > > If we do still need to keep writing partitions this way, can we > > repeat > > the IndexTableSegments in the footer so the entire file doesn't > > have to > > be scanned? > > Yeah, that is what smart tools like bmxtools are doing. If XDCAM requires this amount of partitions then yeah, probably write the index tables twice. That way a smart reader should be able to figure out that it doesn't need to read more than the header, RIP and footer. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()
sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> Signed-off-by: Michael Niedermayer
> ---
> libavformat/mxfdec.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index b85c10bf19..d7cdd22c8a 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -932,7 +932,13 @@ static int mxf_read_cryptographic_context(void
> *arg, AVIOContext *pb, int tag, i
>
> static int mxf_read_strong_ref_array(AVIOContext *pb, UID **refs,
> int *count)
> {
> - *count = avio_rb32(pb);
> + unsigned c = avio_rb32(pb);
not uint32_t?
> +
> + //avio_read() used int
> + if (c > INT_MAX / sizeof(UID))
> + return AVERROR_PATCHWELCOME;
> + *count = c;
> +
This should already be caught by av_calloc(), no?
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxfenc: allow more bits for variable part in uuid generation
mån 2022-03-14 klockan 20:57 +0100 skrev Marton Balint:
>
>
> On Mon, 14 Mar 2022, Tomas Härdin wrote:
>
> > mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint:
> > > Also make sure we do not change the product UID.
> > >
> > > Signed-off-by: Marton Balint
> > > ---
> > > libavformat/mxfenc.c | 9 +
> > > tests/ref/fate/copy-trac4914 | 2 +-
> > > tests/ref/fate/mxf-d10-user-comments | 6 +++---
> > > tests/ref/fate/mxf-opatom-user-comments | 2 +-
> > > tests/ref/fate/mxf-reel_name | 2 +-
> > > tests/ref/fate/mxf-user-comments | 2 +-
> > > tests/ref/fate/time_base | 2 +-
> > > tests/ref/lavf/mxf | 6 +++---
> > > tests/ref/lavf/mxf_d10 | 2 +-
> > > tests/ref/lavf/mxf_dv25 | 2 +-
> > > tests/ref/lavf/mxf_dvcpro50 | 2 +-
> > > tests/ref/lavf/mxf_opatom | 2 +-
> > > tests/ref/lavf/mxf_opatom_audio | 2 +-
> > > 13 files changed, 21 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > index 1e87dc6111..ba8e7babfb 100644
> > > --- a/libavformat/mxfenc.c
> > > +++ b/libavformat/mxfenc.c
> > > @@ -227,7 +227,8 @@ static const UID mxf_d10_container_uls[] = {
> > > {
> > > 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,
> > > 0x01
> > > ,0x06,0x01 }, // D-10 525/50 NTSC 30mb/s
> > > };
> > >
> > > -static const uint8_t uuid_base[] = {
> > > 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd };
> > > +static const uint8_t product_uid[] = {
> > > 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd,0x00,
> > > 0x0c
> > > ,0x00,0x02};
> >
> > Maybe use Identification instead of 0x0C.
>
> Actually I'd rather keep it 0x0C, Identification value might change
> (if
> MXFMetadataSetType enum is reordered in mxf.h), and we don't want
> ProductUID to change even then...
Good point
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxfenc: allow more bits for variable part in uuid generation
mån 2022-03-14 klockan 19:49 +0100 skrev Marton Balint:
> Also make sure we do not change the product UID.
>
> Signed-off-by: Marton Balint
> ---
> libavformat/mxfenc.c | 9 +
> tests/ref/fate/copy-trac4914 | 2 +-
> tests/ref/fate/mxf-d10-user-comments | 6 +++---
> tests/ref/fate/mxf-opatom-user-comments | 2 +-
> tests/ref/fate/mxf-reel_name | 2 +-
> tests/ref/fate/mxf-user-comments | 2 +-
> tests/ref/fate/time_base | 2 +-
> tests/ref/lavf/mxf | 6 +++---
> tests/ref/lavf/mxf_d10 | 2 +-
> tests/ref/lavf/mxf_dv25 | 2 +-
> tests/ref/lavf/mxf_dvcpro50 | 2 +-
> tests/ref/lavf/mxf_opatom | 2 +-
> tests/ref/lavf/mxf_opatom_audio | 2 +-
> 13 files changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 1e87dc6111..ba8e7babfb 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -227,7 +227,8 @@ static const UID mxf_d10_container_uls[] = {
> {
> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x01
> ,0x06,0x01 }, // D-10 525/50 NTSC 30mb/s
> };
>
> -static const uint8_t uuid_base[] = {
> 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd };
> +static const uint8_t product_uid[] = {
> 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff,0x29,0xbd,0x00,0x0c
> ,0x00,0x02};
Maybe use Identification instead of 0x0C.
> +static const uint8_t uuid_base[] = {
> 0xAD,0xAB,0x44,0x24,0x2f,0x25,0x4d,0xc7,0x92,0xff };
> static const uint8_t umid_ul[] = {
> 0x06,0x0A,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x01,0x0D,0x00,0x13 };
>
> /**
> @@ -424,9 +425,9 @@ typedef struct MXFContext {
>
> static void mxf_write_uuid(AVIOContext *pb, enum MXFMetadataSetType
> type, int value)
> {
> - avio_write(pb, uuid_base, 12);
> + avio_write(pb, uuid_base, 10);
> avio_wb16(pb, type);
> - avio_wb16(pb, value);
> + avio_wb32(pb, value);
> }
>
> static void mxf_write_umid(AVFormatContext *s, int type)
> @@ -797,7 +798,7 @@ static void
> mxf_write_identification(AVFormatContext *s)
>
> // write product uid
> mxf_write_local_tag(s, 16, 0x3C05);
> - mxf_write_uuid(pb, Identification, 2);
> + avio_write(pb, product_uid, 16);
For those wondering, the purpose of this not using mxf_write_uuid() is
likely to keep ProductUID the same after this patch. This is of course
good if a bit ugly since the calls with 0 and 1 are still there. Oh
well.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6 1/1] avformat: Add IPFS protocol support.
> + // Test $IPFS_GATEWAY.
> + if (getenv("IPFS_GATEWAY") != NULL) {
> + snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> + getenv("IPFS_GATEWAY"));
might want to error check this one
> + ret = 1;
> + goto err;
> + } else
> + av_log(h, AV_LOG_DEBUG, "$IPFS_GATEWAY is empty.\n");
> +
> + // We need to know the IPFS folder to - eventually - read the
> contents of
> + // the "gateway" file which would tell us the gateway to use.
> + if (getenv("IPFS_PATH") == NULL) {
> + av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> +
> + // Try via the home folder.
> + if (getenv("HOME") == NULL) {
> + av_log(h, AV_LOG_ERROR, "$HOME appears to be empty.\n");
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
> +
> + // Verify the composed path fits.
> + if (snprintf(ipfs_full_data_folder,
> sizeof(ipfs_full_data_folder),
> + "%s/.ipfs/", getenv("HOME")) >
> sizeof(ipfs_full_data_folder)) {
>= not > since snprintf() returns the number of character written sans
the terminating NUL
> + av_log(h, AV_LOG_ERROR, "The IPFS data path exceeds the
> max path length (%li)\n", sizeof(ipfs_full_data_folder));
> + ret = AVERROR(EINVAL);
> + goto err;
> + }
> +
> + // Stat the folder.
> + // It should exist in a default IPFS setup when run as local
> user.
> +#ifndef _WIN32
> + stat_ret = stat(ipfs_full_data_folder, );
> +#else
> + stat_ret = win32_stat(ipfs_full_data_folder, );
> +#endif
> + if (stat_ret < 0) {
> + av_log(h, AV_LOG_INFO, "Unable to find IPFS folder. We
> tried:\n");
> + av_log(h, AV_LOG_INFO, "- $IPFS_PATH, which was
> empty.\n");
> + av_log(h, AV_LOG_INFO, "- $HOME/.ipfs (full uri: %s)
> which doesn't exist.\n", ipfs_full_data_folder);
> + ret = AVERROR(ENOENT);
> + goto err;
> + }
> + } else
> + snprintf(ipfs_full_data_folder,
> sizeof(ipfs_full_data_folder), "%s",
> + getenv("IPFS_PATH"));
not checked
> +
> + // Copy the fully composed gateway path into ipfs_gateway_file.
> + if (snprintf(ipfs_gateway_file, sizeof(gateway_file_data),
> "%sgateway",
> + ipfs_full_data_folder) > sizeof(ipfs_gateway_file))
> {
>=
> + // At this point gateway_file_data contains at least something.
> + // Copy it into c->gateway_buffer.
> + if (snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> + gateway_file_data) > 0) {
> + ret = 1;
> + goto err;
> + } else
> + av_log(h, AV_LOG_DEBUG, "Unknown error in the IPFS gateway
> file.\n");
why not read directly into c->gateway_buffer?
> + // Pppulate c->gateway_buffer with whatever is in c->gateway
> + if (c->gateway != NULL)
> + snprintf(c->gateway_buffer, sizeof(c->gateway_buffer), "%s",
> c->gateway);
> + else
> + c->gateway_buffer[0] = '\0';
> +
> + // Only do the auto detection logic if the gateway_buffer is
> empty
> + if (c->gateway_buffer[0] == '\0') {
these two ifs can be rolled together
> + // Concatenate the url.
> + // This ends up with something like:
> http://localhost:8080/ipfs/Qm.
> + fulluri = av_asprintf("%s%s%s", c->gateway_buffer,
> + (is_ipns) ? "ipns/" : "ipfs/",
> + ipfs_cid);
it is here that I mean you can stick in the / if necessary. that would
make the code much simpler
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 1/1] avformat: Add IPFS protocol support.
> + IPFSGatewayContext *c = h->priv_data;
> + char ipfs_full_data_folder[PATH_MAX];
> + char ipfs_gateway_file[PATH_MAX];
> + struct stat st;
> + int stat_ret = 0;
> + int ret = AVERROR(EINVAL);
> + FILE *gateway_file = NULL;
> +
> + // Set the first character of c->gateway_buffer to 0.
> + c->gateway_buffer[0] = '\0';
unnecessary
> +
> + // Test $IPFS_GATEWAY.
> + if (getenv("IPFS_GATEWAY") != NULL) {
> + if (snprintf(c->gateway_buffer, sizeof(c->gateway_buffer),
> "%s",
> + getenv("IPFS_GATEWAY")) >= sizeof(c-
> >gateway_buffer)) {
> + av_log(h, AV_LOG_ERROR, "The IPFS_GATEWAY environment
> variable exceeds the maximum length. We allow a max of %li
> characters\n", sizeof(c->gateway_buffer));
nit: seems a bit weird to break the if but not the av_log()
Also this should be %zu not %li
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v7 1/1] avformat: Add IPFS protocol support.
tor 2022-02-17 klockan 15:26 +0100 skrev Mark Gaiser:
> >
> > > +
> > > + // Test $IPFS_GATEWAY.
> > > + if (getenv("IPFS_GATEWAY") != NULL) {
> > > + if (snprintf(c->gateway_buffer, sizeof(c-
> > > >gateway_buffer),
> > > "%s",
> > > + getenv("IPFS_GATEWAY")) >= sizeof(c-
> > > > gateway_buffer)) {
> > > + av_log(h, AV_LOG_ERROR, "The IPFS_GATEWAY
> > > environment
> > > variable exceeds the maximum length. We allow a max of %li
> > > characters\n", sizeof(c->gateway_buffer));
> >
> > nit: seems a bit weird to break the if but not the av_log()
> > Also this should be %zu not %li
> >
>
> The compiler doesn't complain about this one.
> How do you know %zu is right? I used this table and it knows nothing
> about
> %z...
> https://www.cplusplus.com/reference/cstdio/printf/
sizeof() returns size_t
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v8 1/1] avformat: Add IPFS protocol support.
sön 2022-02-27 klockan 15:29 +0100 skrev Mark Gaiser: > Ping 2 > > I'd really like to get this merged! > This kinda blocks me right now from proceeding with IPFS integration > in > Kodi, MPV and VLC. Implementations in those (who rely on ffmpeg) are > significantly easier once this patch is finally landed in ffmpeg. I'd like to hear at least one other dev chime in on this one /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v5 1/1] avformat: Add IPFS protocol support.
tor 2022-02-10 klockan 02:13 +0100 skrev Mark Gaiser: > This patch adds support for: > - ffplay ipfs:// > - ffplay ipns:// > > IPFS data can be played from so called "ipfs gateways". > A gateway is essentially a webserver that gives access to the > distributed IPFS network. > > This protocol support (ipfs and ipns) therefore translates > ipfs:// and ipns:// to a http:// url. This resulting url is > then handled by the http protocol. It could also be https > depending on the gateway provided. > > To use this protocol, a gateway must be provided. > If you do nothing it will try to find it in your > $HOME/.ipfs/gateway file. The ways to set it manually are: > 1. Define a -gateway to the gateway. > 2. Define $IPFS_GATEWAY with the full http link to the gateway. > 3. Define $IPFS_PATH and point it to the IPFS data path. > 4. Have IPFS running in your local user folder (under $HOME/.ipfs). > > Signed-off-by: Mark Gaiser > --- > configure | 2 + > doc/protocols.texi | 30 > libavformat/Makefile | 2 + > libavformat/ipfsgateway.c | 326 > ++ > libavformat/protocols.c | 2 + > 5 files changed, 362 insertions(+) > create mode 100644 libavformat/ipfsgateway.c > > diff --git a/configure b/configure > index 5b19a35f59..6ff09e7974 100755 > --- a/configure > +++ b/configure > @@ -3585,6 +3585,8 @@ udp_protocol_select="network" > udplite_protocol_select="network" > unix_protocol_deps="sys_un_h" > unix_protocol_select="network" > +ipfs_protocol_select="https_protocol" > +ipns_protocol_select="https_protocol" > > # external library protocols > libamqp_protocol_deps="librabbitmq" > diff --git a/doc/protocols.texi b/doc/protocols.texi > index d207df0b52..7c9c0a4808 100644 > --- a/doc/protocols.texi > +++ b/doc/protocols.texi > @@ -2025,5 +2025,35 @@ decoding errors. > > @end table > > +@section ipfs > + > +InterPlanetary File System (IPFS) protocol support. One can access > files stored > +on the IPFS network through so called gateways. Those are http(s) > endpoints. > +This protocol wraps the IPFS native protocols (ipfs:// and ipns://) > to be send > +to such a gateway. Users can (and should) host their own node which > means this > +protocol will use your local machine gateway to access files on the > IPFS network. > + > +If a user doesn't have a node of their own then the public gateway > dweb.link is > +used by default. > + > +You can use this protocol in 2 ways. Using IPFS: > +@example > +ffplay ipfs://QmbGtJg23skhvFmu9mJiePVByhfzu5rwo74MEkVDYAmF5T > +@end example > + > +Or the IPNS protocol (IPNS is mutable IPFS): > +@example > +ffplay ipns://QmbGtJg23skhvFmu9mJiePVByhfzu5rwo74MEkVDYAmF5T > +@end example > + > +You can also change the gateway to be used: > + > +@table @option > + > +@item gateway > +Defines the gateway to use. When nothing is provided the protocol > will first try > +your local gateway. If that fails dweb.link will be used. > + > +@end table > > @c man end PROTOCOLS > diff --git a/libavformat/Makefile b/libavformat/Makefile > index 3dc6a479cc..4edce8420f 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -656,6 +656,8 @@ OBJS-$(CONFIG_SRTP_PROTOCOL) += > srtpproto.o srtp.o > OBJS-$(CONFIG_SUBFILE_PROTOCOL) += subfile.o > OBJS-$(CONFIG_TEE_PROTOCOL) += teeproto.o tee_common.o > OBJS-$(CONFIG_TCP_PROTOCOL) += tcp.o > +OBJS-$(CONFIG_IPFS_PROTOCOL) += ipfsgateway.o > +OBJS-$(CONFIG_IPNS_PROTOCOL) += ipfsgateway.o > TLS-OBJS-$(CONFIG_GNUTLS) += tls_gnutls.o > TLS-OBJS-$(CONFIG_LIBTLS) += tls_libtls.o > TLS-OBJS-$(CONFIG_MBEDTLS) += tls_mbedtls.o > diff --git a/libavformat/ipfsgateway.c b/libavformat/ipfsgateway.c > new file mode 100644 > index 00..9ebced10c7 > --- /dev/null > +++ b/libavformat/ipfsgateway.c > @@ -0,0 +1,326 @@ > +/* > + * IPFS and IPNS protocol support through IPFS Gateway. > + * Copyright (c) 2022 Mark Gaiser > + * > + * 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 "avformat.h" > +#include "libavutil/avassert.h" > +#include "libavutil/avstring.h" > +#include
Re: [FFmpeg-devel] [PATCH v4 1/1] avformat: Add IPFS protocol support.
ons 2022-02-09 klockan 18:36 +0100 skrev Mark Gaiser: > > > Thank you so much for all the valuable feedback! It's much > appreciated :) > > I'll fix all the still open issues and send another patch mail > somewhere > later this week. > Question though, as this seems to be heading towards the final patch > version. How do you prefer to see the next patch round? > > 1. As it's done currently. You don't see what's different. > 2. As a 1/2 and 2/2 patch where 2/2 would be the a diff on top of 1/2 > with > the review feedback applied. > > Either is fine with me. > But I fear that a split patch mail (so 1/2 with the code as is right > now > and 2/2 with the feedback changes) could potentially waste people's > time if > they comment on something that is fixed in the feedback patch. Submit patches as you want them in the source tree. Writing patches to patches not yet committed makes little sense. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] web/consulting: add myself
From 446777e4333726cb05843ff16ac76e9eb83a5e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= Date: Sun, 2 Jan 2022 15:47:30 +0100 Subject: [PATCH] web/consulting: add myself --- src/consulting | 12 1 file changed, 12 insertions(+) diff --git a/src/consulting b/src/consulting index 01e4995..96a5b28 100644 --- a/src/consulting +++ b/src/consulting @@ -106,6 +106,18 @@ E.g.: + + + Tomas Härdin + +Tomas is located in Umeå, Sweden. +He has worked on FFmpeg since 2009 and has been a maintainer since 2010. +He has expertise in broadcast formats, mainly MXF and AAF, and is also available for general FFmpeg work. +For more information on Tomas' areas of expertise see https://www.haerdin.se/pages/consulting.html;>the consulting page on his website. +You can contact him by email at consult...@haerdin.se or by XMPP at t...@haerdin.se. + + + Steven Liu -- 2.30.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] avformat/codec2: remove surplus include 'memory.h' statement
ons 2022-03-23 klockan 16:26 +1100 skrev Peter Ross: > on glibc memory.h drags in string.h, but codec2 does not use any > str* or mem* functions. additionally, memory.h is not part of the > C99 or POSIX standards. > --- > libavformat/codec2.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/libavformat/codec2.c b/libavformat/codec2.c > index cd0521299c..400c5acbdb 100644 > --- a/libavformat/codec2.c > +++ b/libavformat/codec2.c > @@ -21,7 +21,6 @@ > > #include "config_components.h" > > -#include > #include "libavcodec/codec2utils.h" > #include "libavutil/channel_layout.h" > #include "libavutil/intreadwrite.h" Looks fine /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()
lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer:
> On Mon, Mar 14, 2022 at 08:19:51PM +0100, Tomas Härdin wrote:
> > sön 2022-03-13 klockan 00:52 +0100 skrev Michael Niedermayer:
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > libavformat/mxfdec.c | 8 +++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index b85c10bf19..d7cdd22c8a 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -932,7 +932,13 @@ static int
> > > mxf_read_cryptographic_context(void
> > > *arg, AVIOContext *pb, int tag, i
> > >
> > > static int mxf_read_strong_ref_array(AVIOContext *pb, UID
> > > **refs,
> > > int *count)
> > > {
> > > - *count = avio_rb32(pb);
> > > + unsigned c = avio_rb32(pb);
> >
> > not uint32_t?
>
> we dont need an exact length variable here
Right, we don't support 16-bit machines
>
>
> >
> > > +
> > > + //avio_read() used int
> > > + if (c > INT_MAX / sizeof(UID))
> > > + return AVERROR_PATCHWELCOME;
> > > + *count = c;
> > > +
> >
> > This should already be caught by av_calloc(), no?
>
> the API as in the documentation of av_calloc() does not gurantee
> this.
Yes it does:
The allocated memory will have size `size * nmemb` bytes.
[...]
`NULL` if the block cannot be allocated
> Its bad practice if we write code that depends on some implementation
> of some code in a diferent module/lib
If av_calloc() does not guarantee this then it is useless. It is used
precisely for this all over the place. Are you going to change every
use of av_calloc() in mxfdec in the same way?
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] web/consulting: add myself
sön 2022-03-27 klockan 10:02 +0530 skrev Gyan Doshi: > > > On 2022-03-27 05:15 am, Michael Niedermayer wrote: > > On Thu, Mar 24, 2022 at 03:53:56PM +0100, Tomas Härdin wrote: > > > consulting | 12 > > > 1 file changed, 12 insertions(+) > > > 901f8f38d654798a4fcb579ac2e50c0a443f5843 0001-web-consulting- > > > add-myself.patch > > > From 446777e4333726cb05843ff16ac76e9eb83a5e57 Mon Sep 17 > > > 00:00:00 2001 > > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= > > > Date: Sun, 2 Jan 2022 15:47:30 +0100 > > > Subject: [PATCH] web/consulting: add myself > > > > > > --- > > > src/consulting | 12 > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/src/consulting b/src/consulting > > > index 01e4995..96a5b28 100644 > > > --- a/src/consulting > > > +++ b/src/consulting > > > @@ -106,6 +106,18 @@ E.g.: > > > > > > > > > > > > + > > > + > > > + Tomas Härdin > > > + > > > + Tomas is located in Umeå, Sweden. > > > + He has worked on FFmpeg since 2009 and has been a > > > maintainer since 2010. > > > + He has expertise in broadcast formats, mainly MXF and > > > AAF, and is also available for general FFmpeg work. > > > + For more information on Tomas' areas of expertise see > > href="https://www.haerdin.se/pages/consulting.html;>the > > > consulting page on his website. > > > + You can contact him by email at consult...@haerdin.se or by XMPP at > > href="xmpp:t...@haerdin.se">t...@haerdin.se. > > > + > > > + > > > + > > > > > > > > > Steven Liu > > I applied this not realizing it messes the "table" style up a bit > > maybe you can fix this > > Done. Cheers. I see also that I accidentally submitted this without obfuscating the email, which I had staged but not amended. Will send a small correction patch- /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] web/consulting: Obfuscate email a bit
Trying to keep my biz emails relatively free of spam From 51071ad87c47e0231b3d364adfe78c2ccbf9ad6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= Date: Tue, 29 Mar 2022 15:18:06 +0200 Subject: [PATCH] web/consulting: Obfuscate email a bit --- src/consulting | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/consulting b/src/consulting index 7c81063..c2c78fc 100644 --- a/src/consulting +++ b/src/consulting @@ -114,7 +114,7 @@ E.g.: He has worked on FFmpeg since 2009 and has been a maintainer since 2010. He has expertise in broadcast formats, mainly MXF and AAF, and is also available for general FFmpeg work. For more information on Tomas' areas of expertise see https://www.haerdin.se/pages/consulting.html;>the consulting page on his website. -You can contact him by email at consult...@haerdin.se or by XMPP at t...@haerdin.se. +You can contact him by email at consulting at haerdin dot se or by XMPP at t...@haerdin.se. -- 2.30.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/4] avformat/mxfdec: Check count in mxf_read_strong_ref_array()
sön 2022-03-20 klockan 15:06 +0100 skrev Michael Niedermayer: > On Sun, Mar 20, 2022 at 02:05:41PM +0100, Tomas Härdin wrote: > > lör 2022-03-19 klockan 23:50 +0100 skrev Michael Niedermayer: > [...] > > > > > > > > > > > > > > > + > > > > > + //avio_read() used int > > > > > + if (c > INT_MAX / sizeof(UID)) > > > > > + return AVERROR_PATCHWELCOME; > > > > > + *count = c; > > > > > + > > > > > > > > This should already be caught by av_calloc(), no? > > > > > > the API as in the documentation of av_calloc() does not gurantee > > > this. > > > > Yes it does: > > > > The allocated memory will have size `size * nmemb` bytes. > > [...] > > `NULL` if the block cannot be allocated > > void *av_calloc(size_t nmemb, size_t size) > size_t can be larger than int, so size * nmemb may be larger than > INT_MAX Crap, you're right. This also brings to mind the question why packages_count etc are int rather than unsigned or uint32_t.. Patch is OK then /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/1] avformat: Add IPFS protocol support.
tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser:
>
> +typedef struct Context {
> + AVClass *class;
> + URLContext *inner;
> + char *gateway;
Is there not a maximum length that an HTTP URL can be? At least without
query parameters. That way you avoid dynamic allocations. You'd have to
separate the AVOption from such a buffer in that case, but I think you
have to anyway.
> + if (!ipfs_full_data_folder) {
> + av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> +
> + // Try via the home folder.
> + home_folder = getenv("HOME");
> + ipfs_full_data_folder = av_asprintf("%s/.ipfs/",
> home_folder);
Memory leak. This applies to most if not all av_asprintf() calls.
> +
> + // Stat the folder. It should exist in a default IPFS setup
> when run as local user.
> +#ifndef _WIN32
> + stat_ret = stat(ipfs_full_data_folder, );
> +#else
> + stat_ret = win32_stat(ipfs_full_data_folder, );
> +#endif
Why bother with stat() when you can just check whether fopen()
succeeded?
> +// For now just makes sure that the gateway ends in url we expect.
> Like http://localhost:8080/.
> +// Explicitly with the traling slash.
> +static void ff_sanitize_ipfs_gateway(URLContext *h)
> +{
> + Context *c = h->priv_data;
> + const char last_gateway_char = c->gateway[strlen(c->gateway) -
> 1];
Can strlen(c->gateway) be zero here?
> +static int translate_ipfs_to_http(URLContext *h, const char *uri,
> int flags, AVDictionary **options)
> +{
> + const char *ipfs_cid;
> + const char *protocol_path_suffix = "ipfs/";
> + char *fulluri;
> + int ret;
> + Context *c = h->priv_data;
> + int is_ipfs = (av_strstart(uri, "ipfs://", _cid) ||
> av_strstart(uri, "ipfs:", _cid));
> + int is_ipns = (av_strstart(uri, "ipns://", _cid) ||
> av_strstart(uri, "ipns:", _cid));
https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the
canonical form. No mentioned is made of any ipfs:{CID} form. Incorrect
URLs should be rejected, not silently patched.
Also what happens if c->gateway is "ipfs://[...]"? Infinite recursion?
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
tis 2022-02-01 klockan 22:18 +0100 skrev Mark Gaiser: > > To give you an idea of how it looks. Kodi has so called strm (stream) > files. They can contain an url that can be played. > > Without this patch an ipfs file would look like this: > > http://localhost:8080/ipfs/QmbGtJg23skhvFmu9mJiePVByhfzu5rwo74MEkVDYAmF5T > > With this patch it becomes possible to patch kodi to accept: > ipfs://QmbGtJg23skhvFmu9mJiePVByhfzu5rwo74MEkVDYAmF5T > > In the former case it's gateway specific. In the latter case it's > gateway > agnostic. > > The gateway specific way has a problem. If i translate it to a > gateway then > that url i picked becomes the point to access the file. > If that url goes down, the file isn't playable. Even if the file > might > still be online just fine on the IPFS network. > Imagine you get a file from me and have ipfs running locally. I'm > guessing > you don't like to edit the file you received to fix the gateway part > of the > url, do you? I certainly don't. You translate the URLs on the fly of course, and don't store the translated URLs in the strm. I can almost guarantee you you will need to do this inside Kodi anyway. What if you want to play a playlist stored in IPFS? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
mån 2022-01-31 klockan 23:00 +0100 skrev Mark Gaiser: > On Mon, Jan 31, 2022 at 9:23 PM Tomas Härdin > wrote: > > > mån 2022-01-31 klockan 17:31 +0100 skrev Mark Gaiser: > > > On Mon, Jan 31, 2022 at 4:52 PM Tomas Härdin > > > wrote: > > > > > > > mån 2022-01-31 klockan 14:51 +0100 skrev Mark Gaiser: > > > > > > > > > > There are multiple ways to access files on the IPFS network. > > > > > This > > > > > patch series > > > > > uses the gateway driven way. An IPFS node - by default - > > > > > exposes > > > > > a > > > > > local > > > > > gateway (say http://localhost:8080) which is then used to get > > > > > content > > > > > from IPFS. > > > > > > > > > > > > Perhaps the protocol should be called something other than just > > > > ipfs if > > > > it doesn't actually implement IPFS. Like ipfsgateway. It could > > > > still be > > > > registered to ipfs:// of course, until someone writes a wrapper > > > > for > > > > libipfs. > > > > > > > > > > Do you mean to have it named like "ipfsgateway" as files (and > > > library) but > > > keep the protocol registration of ipfs and ipns? > > > I'm fine with that. The name is only artificial in code anyhow, > > > all > > > that > > > matters are the protocol names. > > > > What I'm really after is if other devs think there might be an > > issue > > once someone goes an implements proper IPFS support > > > > A "proper" implementation is unfeasible for ffmpeg purposes because a > proper implementation would act as an IPFS node. > That means it would: > - spin up > - do it's bootstrapping > - connect to nodes and find new nodes to connect to > - find the CID on the network > - etc... This sounds similar to Tor, except Tor has a much more elegant way of dealing with this stuff by just wrapping ffmpeg with torify. It takes care of all the IPC stuff with tor.service > > > > It strikes me that this borders on incorporating business logic > > within > > lavf. A user could achieve the same thing with a small shell > > script. > > For example adding an alias that inspects calls to ffmpeg and sed:s > > ipfs:// URLs accordingly > > > > That might work but isn't really user friendly. It also doesn't help > for > tools/applications that incorporate ffmpeg to potentially use IPFS > resources. > KODI (when IPFS is merged into ffmpeg) is one such application where > I'll > be adding support for IPFS. > But there are more that could potentially benefit. Think for example > of OBS > studio and blender (i haven't been in contact with them, all i know > is that > they use ffmpeg). This sounds like business logic that should live in Kodi, OBS and Blender respectively. Or better yet, in the operating system itself (read: systemd). How does Kodi handle browsing media stored in IPFS in this case? Does it not already have to be IPFS aware for this to work reasonably well? Come to think of it, why aren't URLs openable as files on Linux? /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
tis 2022-02-01 klockan 11:06 +0100 skrev Michael Niedermayer: > On Mon, Jan 31, 2022 at 09:22:52PM +0100, Tomas Härdin wrote: > [...] > > It strikes me that this borders on incorporating business logic > > within > > lavf. A user could achieve the same thing with a small shell > > script. > > For example adding an alias that inspects calls to ffmpeg and sed:s > > ipfs:// URLs accordingly > > That sounds like a security nightmare Parsing shit in C is a far bigger nightmare I can assure you. The command line can leverage sed and the regex in the URL RFC. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/1] avformat: Add IPFS protocol support.
ons 2022-02-02 klockan 14:56 +0100 skrev Mark Gaiser:
> On Wed, Feb 2, 2022 at 2:21 PM Tomas Härdin
> wrote:
>
> > tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser:
> >
> > >
> > > +typedef struct Context {
> > > + AVClass *class;
> > > + URLContext *inner;
> > > + char *gateway;
> >
> > Is there not a maximum length that an HTTP URL can be? At least
> > without
> > query parameters. That way you avoid dynamic allocations. You'd
> > have to
> > separate the AVOption from such a buffer in that case, but I think
> > you
> > have to anyway.
> >
>
> Could you provide more information on that? Or an example of what you
> mean
> exactly?
> As far as i know there is no hard limit though it's very much advised
> to
> not go above 2048 characters.
You could at least separate the variable that is set by the AVOption
stuff from the buffer that's produced by the URL parsing stuff.
Since you call ffurl_open_whitelist() immediately you could just use
alloca() to allocate memory on the stack. That way you don't have to
worry about deallocation at all *and* you can have it be arbitrarily
large.
If URLs have a spec'd maximum length then a buffer of constant size on
the stack would work.
>
> >
> > > + if (!ipfs_full_data_folder) {
> > > + av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n");
> > > +
> > > + // Try via the home folder.
> > > + home_folder = getenv("HOME");
> > > + ipfs_full_data_folder = av_asprintf("%s/.ipfs/",
> > > home_folder);
> >
> > Memory leak. This applies to most if not all av_asprintf() calls.
> >
>
> Is there an advised way to neatly clean that up?
> Sure, I can add a bunch of av_free calls to clean it up. But there
> are
> places where it's not as straightforward like where the av_asprintf
> was
> done in an if statement. How do I maintain the knowledge that
> av_asprintf
> was used to call av_free later?
goto as the others pointed out
>
> >
> > > +static int translate_ipfs_to_http(URLContext *h, const char
> > > *uri,
> > > int flags, AVDictionary **options)
> > > +{
> > > + const char *ipfs_cid;
> > > + const char *protocol_path_suffix = "ipfs/";
> > > + char *fulluri;
> > > + int ret;
> > > + Context *c = h->priv_data;
> > > + int is_ipfs = (av_strstart(uri, "ipfs://", _cid) ||
> > > av_strstart(uri, "ipfs:", _cid));
> > > + int is_ipns = (av_strstart(uri, "ipns://", _cid) ||
> > > av_strstart(uri, "ipns:", _cid));
> >
> > https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the
> > canonical form. No mentioned is made of any ipfs:{CID} form.
> > Incorrect
> > URLs should be rejected, not silently patched.
> >
>
> I'd like to make a decision here. This current logic (ipfs:// and
> ipfs:,
> same for ipns) is inspired by other protocols that ffmpeg supported.
> I
> simply copied how they work to be consistent.
> Do i:
> 1. keep it as is and be consistent with the rest?
> 2. only allow ipfs:// and ipns://?
>
Which protocols? This laxness in lavf is worrisome. It breeds laxness
in other projects.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
ons 2022-02-02 klockan 14:48 +0100 skrev Michael Niedermayer: > On Tue, Feb 01, 2022 at 05:43:24PM +0100, Tomas Härdin wrote: > > tis 2022-02-01 klockan 11:06 +0100 skrev Michael Niedermayer: > > > On Mon, Jan 31, 2022 at 09:22:52PM +0100, Tomas Härdin wrote: > > > [...] > > > > It strikes me that this borders on incorporating business logic > > > > within > > > > lavf. A user could achieve the same thing with a small shell > > > > script. > > > > For example adding an alias that inspects calls to ffmpeg and > > > > sed:s > > > > ipfs:// URLs accordingly > > > > > > That sounds like a security nightmare > > > > Parsing shit in C is a far bigger nightmare I can assure you. The > > command line can leverage sed and the regex in the URL RFC. > > the problem is if you parse it on the command line and change it > before passing it to ffmpeg. You really need to have ffmpeg and > the command line parse it 100% exactly the same. The word you're looking for is langsec. It is why I harp on about being very strict with what we accept. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/1] avformat: Add IPFS protocol support.
tor 2022-02-03 klockan 18:29 +0100 skrev Mark Gaiser:
>
> +typedef struct IPFSGatewayContext {
> + AVClass *class;
> + URLContext *inner;
> + char *gateway;
Consider two separate variables. One for AVOption and one for the
dynamically allocated string. Or put the latter on the stack.
> +} IPFSGatewayContext;
> +
> +// A best-effort way to find the IPFS gateway.
> +// Only the most appropiate gateway is set. It's not actually
> requested
> +// (http call) to prevent a potential slowdown in startup. A
> potential timeout
> +// is handled by the HTTP protocol.
> +//
> +// Return codes can be:
> +// 1 : A potential gateway is found and set in c->gateway
> +// -1: The IPFS data folder could not be found
> +// -2: The gateway file could not be found
> +// -3: The gateway file is found but empty
> +// -4: $HOME is empty
> +// -9: Unhandled error
What Michael meant with better return codes is using AVERROR_* :)
> +static int populate_ipfs_gateway(URLContext *h)
> +{
> + IPFSGatewayContext *c = h->priv_data;
> + char *ipfs_full_data_folder = NULL;
> + char *ipfs_gateway_file = NULL;
These can be char[PATH_MAX]
> + struct stat st;
> + int stat_ret = 0;
> + int ret = -9;
> + FILE *gateway_file = NULL;
> + char gateway_file_data[1000];
A maximum URL length of 999?
> +
> + // First, test if there already is a path in c->gateway. If it
> is then it
> + // was provided as cli arument and should be used. It takes
> precdence.
> + if (c->gateway != NULL) {
> + ret = 1;
> + goto err;
> + }
> +
> + // Test $IPFS_GATEWAY.
> + if (getenv("IPFS_GATEWAY") != NULL) {
> + av_free(c->gateway);
Useless since c->gateway is NULL
> +
> + // Stat the folder.
> + // It should exist in a default IPFS setup when run as local
> user.
> +#ifndef _WIN32
> + stat_ret = stat(ipfs_full_data_folder, );
> +#else
> + stat_ret = win32_stat(ipfs_full_data_folder, );
> +#endif
Again, there is no reason to stat this. Just try opening the gateway
file directly.
> +
> + // Read a single line (fgets stops at new line mark).
> + fgets(gateway_file_data, sizeof(gateway_file_data) - 1,
> gateway_file);
This can result in gateway_file_data not being NUL terminated
> +
> + // Replace first occurence of end of line to \0
> + gateway_file_data[strcspn(gateway_file_data, "\r\n")] = 0;
What if the file uses \n or no newlines at all?
> +err:
> + if (gateway_file)
> + fclose(gateway_file);
> +
> + av_free(ipfs_full_data_folder);
> + av_free(ipfs_gateway_file);
This is not cleaning up dynamic allocations of c->gateway
> +// -3: The gateway url part (without the protocol) is too short. We
> expect 3
> +// characters minimal. So http://aaa would be the bare minimal.
http://1 is valid I think. It means http://0.0.0.1
> + // Test if the gateway starts with either http:// or https://
> + // The remainder is stored in url_without_protocol
> + if (av_stristart(uri, "http://;, _without_protocol) == 0
> + && av_stristart(uri, "https://;, _without_protocol) ==
> 0) {
> + av_log(h, AV_LOG_ERROR, "The gateway URL didn't start with
> http:// or https:// and is therefore invalid.\n");
> + ret = -2;
> + goto err;
> + }
I guess restricting this to HTTP schemes is OK. Or are there non-HTTP
gateways for this?
> + if (last_gateway_char != '/') {
> + c->gateway = av_asprintf("%s/", c->gateway);
Yet another leak
> // Sanitize the gateway to a format we expect.
> +if (sanitize_ipfs_gateway(h) < 1)
> +goto err;
This will return unset ret, thus leaking data from the stack
> +static int ipfs_close(URLContext *h)
> +{
> + IPFSGatewayContext *c = h->priv_data;
Here is where you'd put any deallocations
The quality of this patch is making me re-affirm what I've already said
viz parsing. bash+sed is superior.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
mån 2022-01-31 klockan 14:51 +0100 skrev Mark Gaiser: > > There are multiple ways to access files on the IPFS network. This > patch series > uses the gateway driven way. An IPFS node - by default - exposes a > local > gateway (say http://localhost:8080) which is then used to get content > from IPFS. Perhaps the protocol should be called something other than just ipfs if it doesn't actually implement IPFS. Like ipfsgateway. It could still be registered to ipfs:// of course, until someone writes a wrapper for libipfs. /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 0/5] Add IPFS and IPNS protocol support
mån 2022-01-31 klockan 17:31 +0100 skrev Mark Gaiser: > On Mon, Jan 31, 2022 at 4:52 PM Tomas Härdin > wrote: > > > mån 2022-01-31 klockan 14:51 +0100 skrev Mark Gaiser: > > > > > > There are multiple ways to access files on the IPFS network. This > > > patch series > > > uses the gateway driven way. An IPFS node - by default - exposes > > > a > > > local > > > gateway (say http://localhost:8080) which is then used to get > > > content > > > from IPFS. > > > > > > Perhaps the protocol should be called something other than just > > ipfs if > > it doesn't actually implement IPFS. Like ipfsgateway. It could > > still be > > registered to ipfs:// of course, until someone writes a wrapper for > > libipfs. > > > > Do you mean to have it named like "ipfsgateway" as files (and > library) but > keep the protocol registration of ipfs and ipns? > I'm fine with that. The name is only artificial in code anyhow, all > that > matters are the protocol names. What I'm really after is if other devs think there might be an issue once someone goes an implements proper IPFS support It strikes me that this borders on incorporating business logic within lavf. A user could achieve the same thing with a small shell script. For example adding an alias that inspects calls to ffmpeg and sed:s ipfs:// URLs accordingly > > Question though. In a V2 patch, would it make sense to squash > everything in > one commit? Just look at what other patch series look like. This is small enough that a single patch is likely enough yes /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4 1/1] avformat: Add IPFS protocol support.
fre 2022-02-04 klockan 15:12 +0100 skrev Mark Gaiser:
> On Fri, Feb 4, 2022 at 12:10 PM Tomas Härdin
> wrote:
>
> > tor 2022-02-03 klockan 18:29 +0100 skrev Mark Gaiser:
> > >
> > > +typedef struct IPFSGatewayContext {
> > > + AVClass *class;
> > > + URLContext *inner;
> > > + char *gateway;
> >
> > Consider two separate variables. One for AVOption and one for the
> > dynamically allocated string. Or put the latter on the stack.
> >
>
> There always needs to be a gateway so why is reusing that variable an
> issue?
> I'm fine splitting it up but I'd like to understand the benefit of it
> as
> currently I don't see that benefit.
Because of the way AVOption memory allocation works
>
> > > +static int populate_ipfs_gateway(URLContext *h)
> > > +{
> > > + IPFSGatewayContext *c = h->priv_data;
> > > + char *ipfs_full_data_folder = NULL;
> > > + char *ipfs_gateway_file = NULL;
> >
> > These can be char[PATH_MAX]
> >
>
> Oke, will do.
> C code question though.
> How do I use av_asprintf on stack arrays like that?
snprintf(). Also be careful with PATH_MAX and +-1 bytes for the NUL.
>
> > Again, there is no reason to stat this. Just try opening the
> > gateway
> > file directly.
> >
>
> This is a folder, not a file.
>
> The other stat that was here too was a file, I replaced that with an
> fopen.
> It smells sketchy to me to (ab)use fopen to check if a folder exists.
> There's stat for that.
You don't need to check whether the folder exists at all. The only
thing that accomplishes is some AV_LOG_DEBUG prints that won't even get
compiled in unless a users builds with -g (I think). It's not sketchy -
it's spec'd behavior.
>
>
> >
> > > +
> > > + // Read a single line (fgets stops at new line mark).
> > > + fgets(gateway_file_data, sizeof(gateway_file_data) - 1,
> > > gateway_file);
> >
> > This can result in gateway_file_data not being NUL terminated
>
>
> > > +
> > > + // Replace first occurence of end of line to \0
> > > + gateway_file_data[strcspn(gateway_file_data, "\r\n")] = 0;
> >
> > What if the file uses \n or no newlines at all?
> >
>
> Right.
> So I guess the fix here is:
> 1. Initialize gateway_file_data so all bytes are zero
> 2. read a line
> 3. set the last byte of gateway_file_data to 0
>
> Now any text in the string will be the gateway.
>
> Is that a proper fix?
Yes always putting a NUL at the end works. You don't need to initialize
with zero in that case. fgets() will NUL terminate except when there's
an error like the line being too long.
>
>
> > > +err:
> > > + if (gateway_file)
> > > + fclose(gateway_file);
> > > +
> > > + av_free(ipfs_full_data_folder);
> > > + av_free(ipfs_gateway_file);
> >
> > This is not cleaning up dynamic allocations of c->gateway
> >
>
> So I should do that in ipfs_close, right?
That's one place to do it yes. I forget whether _close() is called in
case of errors. av_freep() will set the pointer to NULL after freeing
so no double-frees occur.
>
> >
> >
> > > + // Test if the gateway starts with either http:// or
> > > https://
> > > + // The remainder is stored in url_without_protocol
> > > + if (av_stristart(uri, "http://;, _without_protocol) == 0
> > > + && av_stristart(uri, "https://;, _without_protocol)
> > > ==
> > > 0) {
> > > + av_log(h, AV_LOG_ERROR, "The gateway URL didn't start
> > > with
> > > http:// or https:// and is therefore invalid.\n");
> > > + ret = -2;
> > > + goto err;
> > > + }
> >
> > I guess restricting this to HTTP schemes is OK. Or are there non-
> > HTTP
> > gateways for this?
> >
>
> No.
> At least not from the IPFS camp.
> The IPFS software creates a gateway and that is specifically an http
> gateway.
> Users can put that behind a proxy making it (potentially) a https
> gateway
> but that's about it.
I see. I guess if any user puts this stuff behind gopher:// or
something then that's their problem.
>
> >
> > > + if (last_gateway_char != '/') {
> > > + c->gateway = av_asprintf("%s/", c->gateway);
> >
> > Yet another leak
> >
>
> Please tell me how to fix this one.
> As you can see, I need the c->gateway value to copy and add a "/" to
> it.
>
> In C++ this would just be a dead simple append ;)
Ensure there's enough space for '/' and a NUL and just write that to
the end.
snprintf() can do all of this if used appropriately. For example to
conditionally append "/" you can put %s in the format string and the
ternary
needs_slash ? "/" : ""
as the associated argument
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfdec: add avlanguage dependency
lör 2022-02-05 klockan 22:59 +1000 skrev Zane van Iperen: > Signed-off-by: Zane van Iperen > --- > libavformat/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/Makefile b/libavformat/Makefile > index 3dc6a479cc..6566e40cac 100644 > --- a/libavformat/Makefile > +++ b/libavformat/Makefile > @@ -374,7 +374,7 @@ OBJS-$(CONFIG_MTV_DEMUXER) += mtv.o > OBJS-$(CONFIG_MUSX_DEMUXER) += musx.o > OBJS-$(CONFIG_MV_DEMUXER) += mvdec.o > OBJS-$(CONFIG_MVI_DEMUXER) += mvi.o > -OBJS-$(CONFIG_MXF_DEMUXER) += mxfdec.o mxf.o > +OBJS-$(CONFIG_MXF_DEMUXER) += mxfdec.o mxf.o > avlanguage.o > OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o avc.o > OBJS-$(CONFIG_MXG_DEMUXER) += mxg.o > OBJS-$(CONFIG_NC_DEMUXER) += ncdec.o Looks OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 090/281] lxfdec: convert to new channel layout API
ons 2022-01-12 klockan 22:55 -0300 skrev James Almer: > From: Vittorio Giovara > > Signed-off-by: Vittorio Giovara > Signed-off-by: James Almer > --- > libavformat/lxfdec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libavformat/lxfdec.c b/libavformat/lxfdec.c > index 7889abb2c9..9e0d265f6a 100644 > --- a/libavformat/lxfdec.c > +++ b/libavformat/lxfdec.c > @@ -281,7 +281,8 @@ static int lxf_read_header(AVFormatContext *s) > > st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; > st->codecpar->sample_rate = LXF_SAMPLERATE; > - st->codecpar->channels = lxf->channels; > + st->codecpar->ch_layout.order = AV_CHANNEL_ORDER_UNSPEC; > + st->codecpar->ch_layout.nb_channels = lxf->channels; > > avpriv_set_pts_info(st, 64, 1, st->codecpar->sample_rate); > } Looks OK /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 109/281] mxf: convert to new channel layout API
ons 2022-01-12 klockan 22:56 -0300 skrev James Almer: > From: Vittorio Giovara > > Signed-off-by: Vittorio Giovara > Signed-off-by: Anton Khirnov > Signed-off-by: James Almer > --- > libavformat/mxfdec.c | 34 +- > libavformat/mxfenc.c | 20 +++- > 2 files changed, 32 insertions(+), 22 deletions(-) Looks OK with the reservation that I haven't looked at the new API /Tomas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] libavcodec/cinepakenc: Mark no-skip frames as keyframes
From 442bf0fbba4e13aa7d05ff5c30b07d4c94a8182c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?=
Date: Sat, 9 Apr 2022 14:38:47 +0200
Subject: [PATCH 2/3] libavcodec/cinepakenc: Mark no-skip frames as keyframes
Reset curframe whenever we generate a keyframe.
Use -g instead of -keyint_min.
---
libavcodec/cinepakenc.c | 50 +++-
tests/ref/vsynth/vsynth1-cinepak | 8 ++---
tests/ref/vsynth/vsynth2-cinepak | 8 ++---
tests/ref/vsynth/vsynth_lena-cinepak | 6 ++--
4 files changed, 46 insertions(+), 26 deletions(-)
diff --git a/libavcodec/cinepakenc.c b/libavcodec/cinepakenc.c
index 8b32c02780..6cfe8de200 100644
--- a/libavcodec/cinepakenc.c
+++ b/libavcodec/cinepakenc.c
@@ -113,7 +113,7 @@ typedef struct CinepakEncContext {
enum AVPixelFormat pix_fmt;
int w, h;
int frame_buf_size;
-int curframe, keyint;
+int curframe;
AVLFG randctx;
uint64_t lambda;
int *codebook_input;
@@ -215,7 +215,6 @@ static av_cold int cinepak_encode_init(AVCodecContext *avctx)
s->h = avctx->height;
s->frame_buf_size = frame_buf_size;
s->curframe = 0;
-s->keyint = avctx->keyint_min;
s->pix_fmt= avctx->pix_fmt;
// set up AVFrames
@@ -835,8 +834,7 @@ static void calculate_skip_errors(CinepakEncContext *s, int h,
}
}
-static void write_strip_header(CinepakEncContext *s, int y, int h, int keyframe,
- unsigned char *buf, int strip_size)
+static void write_strip_keyframe(unsigned char *buf, int keyframe)
{
// actually we are exclusively using intra strip coding (how much can we win
// otherwise? how to choose which part of a codebook to update?),
@@ -844,6 +842,12 @@ static void write_strip_header(CinepakEncContext *s, int y, int h, int keyframe,
// (besides, the logic here used to be inverted: )
//buf[0] = keyframe ? 0x11: 0x10;
buf[0] = keyframe ? 0x10 : 0x11;
+}
+
+static void write_strip_header(CinepakEncContext *s, int y, int h, int keyframe,
+ unsigned char *buf, int strip_size)
+{
+write_strip_keyframe(buf, keyframe);
AV_WB24([1], strip_size + STRIP_HEADER_SIZE);
// AV_WB16([4], y); /* using absolute y values works -- rl */
AV_WB16([4], 0); /* using relative values works as well -- rl */
@@ -857,7 +861,7 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe,
uint8_t *last_data[4], int last_linesize[4],
uint8_t *data[4], int linesize[4],
uint8_t *scratch_data[4], int scratch_linesize[4],
-unsigned char *buf, int64_t *best_score)
+unsigned char *buf, int64_t *best_score, int *no_skip)
{
int64_t score = 0;
int best_size = 0;
@@ -973,6 +977,9 @@ static int rd_strip(CinepakEncContext *s, int y, int h, int keyframe,
scratch_data, scratch_linesize,
last_data, last_linesize, ,
s->strip_buf + STRIP_HEADER_SIZE);
+// in theory we could have MODE_MC without ENC_SKIP,
+// but MODE_V1_V4 will always be more efficient
+*no_skip = info.mode != MODE_MC;
write_strip_header(s, y, h, keyframe, s->strip_buf, best_size);
}
@@ -999,13 +1006,13 @@ static int write_cvid_header(CinepakEncContext *s, unsigned char *buf,
}
static int rd_frame(CinepakEncContext *s, const AVFrame *frame,
-int isakeyframe, unsigned char *buf, int buf_size)
+int isakeyframe, unsigned char *buf, int buf_size, int *got_keyframe)
{
int num_strips, strip, i, y, nexty, size, temp_size, best_size;
uint8_t *last_data[4], *data[4], *scratch_data[4];
int last_linesize[4], linesize[4], scratch_linesize[4];
int64_t best_score = 0, score, score_temp;
-int best_nstrips;
+int best_nstrips, best_strip_offsets[MAX_STRIPS];
if (s->pix_fmt == AV_PIX_FMT_RGB24) {
int x;
@@ -1064,12 +1071,15 @@ static int rd_frame(CinepakEncContext *s, const AVFrame *frame,
// would be nice but quite certainly incompatible with vintage players:
// support encoding zero strips (meaning skip the whole frame)
for (num_strips = s->min_strips; num_strips <= s->max_strips && num_strips <= s->h / MB_SIZE; num_strips++) {
+int strip_offsets[MAX_STRIPS];
+int all_no_skip = 1;
score = 0;
size = 0;
for (y = 0, strip = 1; y < s->h; strip++, y = nexty) {
-int strip_height;
+int strip_height, no_skip;
+strip_offsets[strip-1] = size + CVID_HEADER_SIZE;
nexty = strip * s->h / num_strips; // <= s->h
// make nexty the next multiple of 4 if not already there
[FFmpeg-devel] [PATCH 1/3] doc/encoders.texi: Document cinepak encoder
From 3cca353f177b9b91e1472fa60a6295e576142c59 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?=
Date: Sat, 9 Apr 2022 10:21:31 +0200
Subject: [PATCH 1/3] doc/encoders.texi: Document cinepak encoder
---
doc/encoders.texi | 48 +++
1 file changed, 48 insertions(+)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 806cc430d4..1adfe382a4 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1271,6 +1271,54 @@ follows.
A64 / Commodore 64 multicolor charset encoder. @code{a64_multi5} is extended with 5th color (colram).
+@section Cinepak
+
+Cinepak aka CVID encoder.
+Compatible with Windows 3.1 and vintage MacOS.
+
+@subsection Options
+
+@table @option
+@item keyint_min @var{integer}
+Keyframe interval.
+A keyframe is inserted every @code{-keyint_min}-th frame.
+
+@item q:v @var{integer}
+Quality factor. Lower is better. Higher gives lower bitrate.
+The following table lists bitrates when encoding akiyo_cif.y4m for various values of @code{-q:v} with @code{-keyint_min 100}:
+
+@table @option
+@item @code{-q:v 1} 1918 kb/s
+@item @code{-q:v 2} 1735 kb/s
+@item @code{-q:v 4} 1500 kb/s
+@item @code{-q:v 10} 1064 kb/s
+@item @code{-q:v 20} 826 kb/s
+@item @code{-q:v 40} 553 kb/s
+@item @code{-q:v 100} 394 kb/s
+@item @code{-q:v 200} 311 kb/s
+@item @code{-q:v 400} 266 kb/s
+@item @code{-q:v 1000} 237 kb/s
+@end table
+
+@item max_extra_cb_iterations @var{integer}
+Max extra codebook recalculation passes, more is better and slower.
+
+@item skip_empty_cb @var{boolean}
+Avoid wasting bytes, ignore vintage MacOS decoder.
+
+@item max_strips @var{integer}
+@itemx min_strips @var{integer}
+The minimum and maximum number of strips to use.
+Wider range sometimes improves quality.
+More strips is generally better quality but costs more bits.
+Vintage compatible is 1..3.
+
+@item strip_number_adaptivity @var{integer}
+How much number of strips is allowed to change between frames.
+Higher is better but slower.
+
+@end table
+
@section GIF
GIF image/animation encoder.
--
2.30.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".
