Re: [FFmpeg-devel] [PATCH] avformat/mxf: Establish register of local tags
On Thu, 28 Jan 2021, Tomas Härdin wrote: ons 2021-01-27 klockan 23:50 +0100 skrev Marton Balint: On Wed, 27 Jan 2021, Tomas Härdin wrote: > ons 2021-01-27 klockan 22:24 +0100 skrev Tomas Härdin: > > ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint: > > > On Wed, 27 Jan 2021, Tomas Härdin wrote: > > > > > > > Hi > > > > > > > > Ticket #9079 brought this about. This should prevent accidentally > > > > adding local tags that are not registered in the primer. It also allows > > > > us to omit tags that we know won't be used, in a manner that is more > > > > elegant than the old code. > > > > > > > > The actual meat of this patch is mxf_mark_tag_unused(), > > > > mxf_write_primer_pack(), mxf_write_local_tag() and > > > > ff_mxf_lookup_local_tag() > > > > > > IMHO you should not move the local tags to mxf.c, because only encoding > > > uses them. > > > > > > The only exception where sharing made sense is > > > ff_mxf_mastering_display_local_tags, but that is super ugly that you > > > now lookup them in mxfdec.c based on local tags we assign them for > > > encoding. Not to mention the linear search you use for each lookup... > > > > We could sort them and use a binary search, but I wanted some feedback > > on this idea before going further. There's not terribly many of them > > > > I'd like to avoid having the full ULs twice in the code. The only way I > > can see how to do that is with #defines > > > > > So I suggest you simply duplicate the 4 UL-s to the single local tags > > > array you make and keep them in mxfenc.c, that way you also don't have to > > > specify the array size manually... > > > > That might conflict with Andreas' deduplication efforts. But yeah, the > > thought did occur to me > > Here's an updated patch. Feedback welcome. Thanks, I like this version much more. One comment is that I'd put an assert right into mxf_lookup_local_tag instead of returning NULL if a tag is not found, this way you can remove NULL-check asserts from individual places where mxf_lookup_local_tag is called. Otherwise seems all fine. There's not really anything to av_assert0() on in mxf_lookup_local_tag(). Either way, I'm thinking replacing the return NULL with either av_log(NULL, AV_LOG_PANIC, "Tried to use unregistered local tag 0x%04x\n", tag); abort(); or av_assert0(0 && "Tried to use unregistered local tag"); or maybe av_log(NULL, AV_LOG_PANIC, "Tried to use unregistered local tag 0x%04x\n", tag); av_assert0(0); to avoid explicitly calling abort() I think we usually do av_assert0(0) in this case. I am not sure if the error message is particularly useful, afterall who sees it should be a programmer and file/line is printed by assert anyway, so a comment in the source code before the assert makes more sense to me. Regards, Marton ___ 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] ffprobe: stop printing deprecated fields
On Thu, 28 Jan 2021, Gyan Doshi wrote: On 28-01-2021 05:08 am, Marton Balint wrote: So only timecode, convergence_duration and the pseudopal flag should be removed, because there is no replacement for those. Will timecode still be available from ffprobe after this patchset? Only mpeg2 GOP timecode as stream metadata is affected, and as far as I see it does not even work right now. Regards, Marton ___ 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] ffprobe: stop printing deprecated fields
On 28-01-2021 05:08 am, Marton Balint wrote: So only timecode, convergence_duration and the pseudopal flag should be removed, because there is no replacement for those. Will timecode still be available from ffprobe after this patchset? Regards, Gyan ___ 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 3/3] avformat/mxfenc: prefer to use the configured metadta
On Wed, Jan 27, 2021 at 08:42:40AM +0100, Tobias Rapp wrote: > On 26.01.2021 16:39, Tomas Härdin wrote: > > tis 2021-01-26 klockan 00:34 +0100 skrev Marton Balint: > > > Maybe they should post patches instead of having workarounds? And I > > > explained, libavformat MXF muxer will still be identifiable, but the > > > proper field will be used for identifying the SDK used, not > > > Company/Product but Toolkit/Platform. > > > > I'll just add that I've read the updated version of S377m from 2007 and > > this sounds like the best solution. In short something like this: > > > > CompanyName: FFmpeg > > Product: OP1a Muxer > > Platform: libavformat x.y.z (Debian GNU/Linux 10 (buster)) > > > > If we delete the metadata in ffmpeg_opt.c like the original patch does > > here, and have Platform be the only hardcoded string, then hopefully > > everyone should be relatively happy. So CompanyName and Product can be > > changed from the command line/API but Platform cannot. > > Sounds reasonable to me. I haven't found the s337m freely, so I'm not sure about the Platforma metadata. I think we haven't support the field yet, I guess it's 0x3C08 tag, but I haven't the document in hand so it's not OK to add it by me. > > Regards, > Tobias > > ___ > 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". -- Thanks, Limin Wang ___ 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] ffprobe: stop printing deprecated fields
On 1/27/2021 8:38 PM, Marton Balint wrote:
On Wed, 27 Jan 2021, James Almer wrote:
On 1/27/2021 7:42 PM, Michael Niedermayer wrote:
On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote:
Also drop the sections guarded by #if FF_API*
These macros are private and should not be used by external callers.
---
fftools/ffprobe.c | 34 --
.../ref/fate/concat-demuxer-extended-lavf-mxf | 2 +-
.../fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +-
.../ref/fate/concat-demuxer-simple1-lavf-mxf | 248 +-
.../fate/concat-demuxer-simple1-lavf-mxf_d10 | 144 +++---
tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++--
tests/ref/fate/ffprobe_compact | 34 +-
tests/ref/fate/ffprobe_csv | 34 +-
tests/ref/fate/ffprobe_default | 42 --
tests/ref/fate/ffprobe_flat | 42 --
tests/ref/fate/ffprobe_ini | 42 --
tests/ref/fate/ffprobe_json | 9 -
tests/ref/fate/ffprobe_xml | 6 +-
tests/ref/fate/flcl1905 | 34 +-
...hapqa-extract-nosnappy-to-hapalphaonly-mov | 8 -
.../fate/hapqa-extract-nosnappy-to-hapq-mov | 8 -
tests/ref/fate/hls-fmp4_ac3 | 1 -
tests/ref/fate/mov-aac-2048-priming | 434
+-
tests/ref/fate/mov-init-nonkeyframe | 240 +-
tests/ref/fate/mov-zombie | 134 +++---
tests/ref/fate/mxf-probe-applehdr10 | 10 -
tests/ref/fate/mxf-probe-d10 | 8 -
tests/ref/fate/mxf-probe-dnxhd | 9 -
tests/ref/fate/mxf-probe-dv25 | 10 -
24 files changed, 807 insertions(+), 1030 deletions(-)
This decreases the amount of fields regression tested.
Iam not sure if thats a good idea. Generally more testing is better
These are all deprecated fields, so they will be gone with the bump.
I thought so too, but on a second look ist->dec_ctx is the codec used at
avcodec_open, so that still can be used, and the deprecation #ifs were
put there as a mistake or historically. Overall the way I see it these
fields can and should still be provided:
print_int("coded_width", dec_ctx->coded_width);
print_int("coded_height", dec_ctx->coded_height);
print_int("closed_captions", !!(dec_ctx->properties &
Actually, this is already being done, but it's like you said wrongly
wrapped with a FF_API check because they were also wrongly/superfluously
copied from stream->codec.
So we should be able to remove those assignments altogether and keep the
print_int() lines as is (Just removing the wrappers).
FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
print_q("codec_time_base", dec_ctx->time_base, '/');
And if "max_bit_rate" is converted to use dec_ctx instead of
stream->codec then probably that can stay too.
So only timecode, convergence_duration and the pseudopal flag should be
removed, because there is no replacement for those.
Regards,
Marton
This patch is doing what you mentioned was the correct approach
regarding deprecation removals, which was removing FF_API defines
individually or in small batches per patch, instead of bumping
LIB*_VERSION_MAJOR and disabling them all in one go.
If you prefer, what could be done is wait until ffmpeg 4.4 is branched
out before applying this patch (and probably also 4/4), which i assume
was planned to happen before the bump so distros can get all of last
year's development in their current releases (mainly LTSs) simply by
dropping in the new libraries.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
From: Levi Dooley
There was an assumption in the existing code that indentation would not occur
more than once on the same row.
This was a bad assumption. There are examples of 608 streams which call
handle_pac multiple times on the same row with different indentation.
As the code was before this change, the new indentation would overwrite
existing text with spaces.
These changes make indentation skip over columns instead. Text gets cleared
with spaces on handle_edm.
Instead of relying on the null character, trailing spaces are trimmed off the
end of a row.
This is necessary so that a null character doesn't end up between two words.
Signed-off-by: Levi Dooley
---
libavcodec/ccaption_dec.c | 56 ---
tests/ref/fate/sub-scc| 2 +-
2 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/libavcodec/ccaption_dec.c b/libavcodec/ccaption_dec.c
index a208e19b95..525e010897 100644
--- a/libavcodec/ccaption_dec.c
+++ b/libavcodec/ccaption_dec.c
@@ -352,6 +352,17 @@ static void write_char(CCaptionSubContext *ctx, struct
Screen *screen, char ch)
}
}
+/**
+ * Increment the cursor_column by 1, and ensure that there are no null
characters left behind in the row.
+ */
+static void skip_char(CCaptionSubContext *ctx, struct Screen *screen)
+{
+if (!screen->characters[ctx->cursor_row][ctx->cursor_column])
+write_char(ctx, screen, ' ');
+else
+ctx->cursor_column++;
+}
+
/**
* This function after validating parity bit, also remove it from data pair.
* The first byte doesn't pass parity, we replace it with a solid blank
@@ -459,6 +470,7 @@ static int capture_screen(CCaptionSubContext *ctx)
if (CHECK_FLAG(screen->row_used, i)) {
const char *row = screen->characters[i];
const char *charset = screen->charsets[i];
+
j = 0;
while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN)
j++;
@@ -476,13 +488,19 @@ static int capture_screen(CCaptionSubContext *ctx)
const char *color = screen->colors[i];
const char *charset = screen->charsets[i];
const char *override;
-int x, y, seen_char = 0;
+int x, y, row_end, seen_char = 0;
j = 0;
/* skip leading space */
while (row[j] == ' ' && charset[j] == CCSET_BASIC_AMERICAN && j <
tab)
j++;
+/* skip trailing space */
+row_end = SCREEN_COLUMNS-1;
+while (row_end >= 0 && row[row_end] == ' ' && charset[row_end] ==
CCSET_BASIC_AMERICAN) {
+row_end--;
+}
+
x = ASS_DEFAULT_PLAYRESX * (0.1 + 0.0250 * j);
y = ASS_DEFAULT_PLAYRESY * (0.1 + 0.0533 * i);
av_bprintf(&ctx->buffer[bidx], "{\\an7}{\\pos(%d,%d)}", x, y);
@@ -490,7 +508,7 @@ static int capture_screen(CCaptionSubContext *ctx)
for (; j < SCREEN_COLUMNS; j++) {
const char *e_tag = "", *s_tag = "", *c_tag = "", *b_tag = "";
-if (row[j] == 0)
+if (j > row_end || row[j] == 0)
break;
if (prev_font != font[j]) {
@@ -624,7 +642,8 @@ static void handle_textattr(CCaptionSubContext *ctx,
uint8_t hi, uint8_t lo)
ctx->cursor_font = pac2_attribs[i][1];
SET_FLAG(screen->row_used, ctx->cursor_row);
-write_char(ctx, screen, ' ');
+
+skip_char(ctx, screen);
}
static void handle_pac(CCaptionSubContext *ctx, uint8_t hi, uint8_t lo)
@@ -644,13 +663,13 @@ static void handle_pac(CCaptionSubContext *ctx, uint8_t
hi, uint8_t lo)
lo &= 0x1f;
ctx->cursor_row = row_map[index] - 1;
-ctx->cursor_color = pac2_attribs[lo][0];
+ctx->cursor_color = pac2_attribs[lo][0];
ctx->cursor_font = pac2_attribs[lo][1];
ctx->cursor_charset = CCSET_BASIC_AMERICAN;
ctx->cursor_column = 0;
indent = pac2_attribs[lo][2];
for (i = 0; i < indent; i++) {
-write_char(ctx, screen, ' ');
+skip_char(ctx, screen);
}
}
@@ -667,6 +686,14 @@ static int handle_edm(CCaptionSubContext *ctx)
screen->row_used = 0;
ctx->bg_color = CCCOL_BLACK;
+for (int i = 0; i < SCREEN_ROWS+1; ++i) {
+memset(screen->characters[i], ' ', SCREEN_COLUMNS);
+memset(screen->colors[i], CCCOL_WHITE, SCREEN_COLUMNS);
+memset(screen->bgs[i],CCCOL_BLACK, SCREEN_COLUMNS);
+memset(screen->charsets[i], CCSET_BASIC_AMERICAN, SCREEN_COLUMNS);
+memset(screen->fonts[i], CCFONT_REGULAR, SCREEN_COLUMNS);
+}
+
// In realtime mode, emit an empty caption so the last one doesn't
// stay on the screen.
if (ctx->real_time)
@@ -687,6 +714,7 @@ static int handle_eoc(CCaptionSubContext *ctx)
ret = handle_edm(ctx);
ctx->cursor_column = 0;
+ctx->cursor_row = 0;
// In realtime mode, we display the buffered co
Re: [FFmpeg-devel] [PATCH 3/4] ffprobe: stop printing deprecated fields
On Wed, 27 Jan 2021, James Almer wrote:
On 1/27/2021 7:42 PM, Michael Niedermayer wrote:
On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote:
Also drop the sections guarded by #if FF_API*
These macros are private and should not be used by external callers.
---
fftools/ffprobe.c | 34 --
.../ref/fate/concat-demuxer-extended-lavf-mxf | 2 +-
.../fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +-
.../ref/fate/concat-demuxer-simple1-lavf-mxf | 248 +-
.../fate/concat-demuxer-simple1-lavf-mxf_d10 | 144 +++---
tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++--
tests/ref/fate/ffprobe_compact| 34 +-
tests/ref/fate/ffprobe_csv| 34 +-
tests/ref/fate/ffprobe_default| 42 --
tests/ref/fate/ffprobe_flat | 42 --
tests/ref/fate/ffprobe_ini| 42 --
tests/ref/fate/ffprobe_json | 9 -
tests/ref/fate/ffprobe_xml| 6 +-
tests/ref/fate/flcl1905 | 34 +-
...hapqa-extract-nosnappy-to-hapalphaonly-mov | 8 -
.../fate/hapqa-extract-nosnappy-to-hapq-mov | 8 -
tests/ref/fate/hls-fmp4_ac3 | 1 -
tests/ref/fate/mov-aac-2048-priming | 434 +-
tests/ref/fate/mov-init-nonkeyframe | 240 +-
tests/ref/fate/mov-zombie | 134 +++---
tests/ref/fate/mxf-probe-applehdr10 | 10 -
tests/ref/fate/mxf-probe-d10 | 8 -
tests/ref/fate/mxf-probe-dnxhd| 9 -
tests/ref/fate/mxf-probe-dv25 | 10 -
24 files changed, 807 insertions(+), 1030 deletions(-)
This decreases the amount of fields regression tested.
Iam not sure if thats a good idea. Generally more testing is better
These are all deprecated fields, so they will be gone with the bump.
I thought so too, but on a second look ist->dec_ctx is the codec used at
avcodec_open, so that still can be used, and the deprecation #ifs were put
there as a mistake or historically. Overall the way I see it these fields
can and should still be provided:
print_int("coded_width", dec_ctx->coded_width);
print_int("coded_height", dec_ctx->coded_height);
print_int("closed_captions", !!(dec_ctx->properties &
FF_CODEC_PROPERTY_CLOSED_CAPTIONS));
print_q("codec_time_base", dec_ctx->time_base, '/');
And if "max_bit_rate" is converted to use dec_ctx instead of stream->codec
then probably that can stay too.
So only timecode, convergence_duration and the pseudopal flag should be
removed, because there is no replacement for those.
Regards,
Marton
This patch is doing what you mentioned was the correct approach
regarding deprecation removals, which was removing FF_API defines
individually or in small batches per patch, instead of bumping
LIB*_VERSION_MAJOR and disabling them all in one go.
If you prefer, what could be done is wait until ffmpeg 4.4 is branched
out before applying this patch (and probably also 4/4), which i assume
was planned to happen before the bump so distros can get all of last
year's development in their current releases (mainly LTSs) simply by
dropping in the new libraries.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: Establish register of local tags
ons 2021-01-27 klockan 23:50 +0100 skrev Marton Balint: > > On Wed, 27 Jan 2021, Tomas Härdin wrote: > > > ons 2021-01-27 klockan 22:24 +0100 skrev Tomas Härdin: > > > ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint: > > > > On Wed, 27 Jan 2021, Tomas Härdin wrote: > > > > > > > > > Hi > > > > > > > > > > Ticket #9079 brought this about. This should prevent accidentally > > > > > adding local tags that are not registered in the primer. It also > > > > > allows > > > > > us to omit tags that we know won't be used, in a manner that is more > > > > > elegant than the old code. > > > > > > > > > > The actual meat of this patch is mxf_mark_tag_unused(), > > > > > mxf_write_primer_pack(), mxf_write_local_tag() and > > > > > ff_mxf_lookup_local_tag() > > > > > > > > IMHO you should not move the local tags to mxf.c, because only encoding > > > > uses them. > > > > > > > > The only exception where sharing made sense is > > > > ff_mxf_mastering_display_local_tags, but that is super ugly that you > > > > now lookup them in mxfdec.c based on local tags we assign them for > > > > encoding. Not to mention the linear search you use for each lookup... > > > > > > We could sort them and use a binary search, but I wanted some feedback > > > on this idea before going further. There's not terribly many of them > > > > > > I'd like to avoid having the full ULs twice in the code. The only way I > > > can see how to do that is with #defines > > > > > > > So I suggest you simply duplicate the 4 UL-s to the single local tags > > > > array you make and keep them in mxfenc.c, that way you also don't have > > > > to > > > > specify the array size manually... > > > > > > That might conflict with Andreas' deduplication efforts. But yeah, the > > > thought did occur to me > > > > Here's an updated patch. Feedback welcome. > > Thanks, I like this version much more. One comment is that I'd put an > assert right into mxf_lookup_local_tag instead of returning NULL if a tag > is not found, this way you can remove NULL-check asserts from individual > places where mxf_lookup_local_tag is called. Otherwise seems all fine. There's not really anything to av_assert0() on in mxf_lookup_local_tag(). Either way, I'm thinking replacing the return NULL with either av_log(NULL, AV_LOG_PANIC, "Tried to use unregistered local tag 0x%04x\n", tag); abort(); or av_assert0(0 && "Tried to use unregistered local tag"); or maybe av_log(NULL, AV_LOG_PANIC, "Tried to use unregistered local tag 0x%04x\n", tag); av_assert0(0); to avoid explicitly calling abort() /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] ffprobe: stop printing deprecated fields
On 1/27/2021 7:42 PM, Michael Niedermayer wrote: On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote: Also drop the sections guarded by #if FF_API* These macros are private and should not be used by external callers. --- fftools/ffprobe.c | 34 -- .../ref/fate/concat-demuxer-extended-lavf-mxf | 2 +- .../fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +- .../ref/fate/concat-demuxer-simple1-lavf-mxf | 248 +- .../fate/concat-demuxer-simple1-lavf-mxf_d10 | 144 +++--- tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++-- tests/ref/fate/ffprobe_compact| 34 +- tests/ref/fate/ffprobe_csv| 34 +- tests/ref/fate/ffprobe_default| 42 -- tests/ref/fate/ffprobe_flat | 42 -- tests/ref/fate/ffprobe_ini| 42 -- tests/ref/fate/ffprobe_json | 9 - tests/ref/fate/ffprobe_xml| 6 +- tests/ref/fate/flcl1905 | 34 +- ...hapqa-extract-nosnappy-to-hapalphaonly-mov | 8 - .../fate/hapqa-extract-nosnappy-to-hapq-mov | 8 - tests/ref/fate/hls-fmp4_ac3 | 1 - tests/ref/fate/mov-aac-2048-priming | 434 +- tests/ref/fate/mov-init-nonkeyframe | 240 +- tests/ref/fate/mov-zombie | 134 +++--- tests/ref/fate/mxf-probe-applehdr10 | 10 - tests/ref/fate/mxf-probe-d10 | 8 - tests/ref/fate/mxf-probe-dnxhd| 9 - tests/ref/fate/mxf-probe-dv25 | 10 - 24 files changed, 807 insertions(+), 1030 deletions(-) This decreases the amount of fields regression tested. Iam not sure if thats a good idea. Generally more testing is better These are all deprecated fields, so they will be gone with the bump. This patch is doing what you mentioned was the correct approach regarding deprecation removals, which was removing FF_API defines individually or in small batches per patch, instead of bumping LIB*_VERSION_MAJOR and disabling them all in one go. If you prefer, what could be done is wait until ffmpeg 4.4 is branched out before applying this patch (and probably also 4/4), which i assume was planned to happen before the bump so distros can get all of last year's development in their current releases (mainly LTSs) simply by dropping in the new libraries. ___ 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] ffprobe: stop printing deprecated fields
On Wed, Jan 27, 2021 at 11:43 PM Michael Niedermayer wrote: > On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote: > > Also drop the sections guarded by #if FF_API* > > These macros are private and should not be used by external callers. > > --- > > fftools/ffprobe.c | 34 -- > > .../ref/fate/concat-demuxer-extended-lavf-mxf | 2 +- > > .../fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +- > > .../ref/fate/concat-demuxer-simple1-lavf-mxf | 248 +- > > .../fate/concat-demuxer-simple1-lavf-mxf_d10 | 144 +++--- > > tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++-- > > tests/ref/fate/ffprobe_compact| 34 +- > > tests/ref/fate/ffprobe_csv| 34 +- > > tests/ref/fate/ffprobe_default| 42 -- > > tests/ref/fate/ffprobe_flat | 42 -- > > tests/ref/fate/ffprobe_ini| 42 -- > > tests/ref/fate/ffprobe_json | 9 - > > tests/ref/fate/ffprobe_xml| 6 +- > > tests/ref/fate/flcl1905 | 34 +- > > ...hapqa-extract-nosnappy-to-hapalphaonly-mov | 8 - > > .../fate/hapqa-extract-nosnappy-to-hapq-mov | 8 - > > tests/ref/fate/hls-fmp4_ac3 | 1 - > > tests/ref/fate/mov-aac-2048-priming | 434 +- > > tests/ref/fate/mov-init-nonkeyframe | 240 +- > > tests/ref/fate/mov-zombie | 134 +++--- > > tests/ref/fate/mxf-probe-applehdr10 | 10 - > > tests/ref/fate/mxf-probe-d10 | 8 - > > tests/ref/fate/mxf-probe-dnxhd| 9 - > > tests/ref/fate/mxf-probe-dv25 | 10 - > > 24 files changed, 807 insertions(+), 1030 deletions(-) > > This decreases the amount of fields regression tested. > Iam not sure if thats a good idea. Generally more testing is better > > How testing deprecated stuff is useful? > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The educated differ from the uneducated as much as the living from the > dead. -- Aristotle > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: add Coding Equations and Color Primaries to local tags
On Wed, 27 Jan 2021, Tomas Härdin wrote: ons 2021-01-27 klockan 22:07 +0100 skrev Marton Balint: Fixes ticket #9079. Signed-off-by: Marton Balint --- libavformat/mxfenc.c| 2 ++ tests/ref/fate/copy-trac4914| 2 +- tests/ref/fate/mxf-d10-user-comments| 2 +- 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, 16 insertions(+), 14 deletions(-) Looks good to me Thanks, applied. Regards, Marton ___ 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: Establish register of local tags
On Wed, 27 Jan 2021, Tomas Härdin wrote: ons 2021-01-27 klockan 22:24 +0100 skrev Tomas Härdin: ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint: On Wed, 27 Jan 2021, Tomas Härdin wrote: Hi Ticket #9079 brought this about. This should prevent accidentally adding local tags that are not registered in the primer. It also allows us to omit tags that we know won't be used, in a manner that is more elegant than the old code. The actual meat of this patch is mxf_mark_tag_unused(), mxf_write_primer_pack(), mxf_write_local_tag() and ff_mxf_lookup_local_tag() IMHO you should not move the local tags to mxf.c, because only encoding uses them. The only exception where sharing made sense is ff_mxf_mastering_display_local_tags, but that is super ugly that you now lookup them in mxfdec.c based on local tags we assign them for encoding. Not to mention the linear search you use for each lookup... We could sort them and use a binary search, but I wanted some feedback on this idea before going further. There's not terribly many of them I'd like to avoid having the full ULs twice in the code. The only way I can see how to do that is with #defines So I suggest you simply duplicate the 4 UL-s to the single local tags array you make and keep them in mxfenc.c, that way you also don't have to specify the array size manually... That might conflict with Andreas' deduplication efforts. But yeah, the thought did occur to me Here's an updated patch. Feedback welcome. Thanks, I like this version much more. One comment is that I'd put an assert right into mxf_lookup_local_tag instead of returning NULL if a tag is not found, this way you can remove NULL-check asserts from individual places where mxf_lookup_local_tag is called. Otherwise seems all fine. Regards, Marton ___ 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] ffprobe: stop printing deprecated fields
On Tue, Jan 26, 2021 at 06:01:01PM +0100, Anton Khirnov wrote: > Also drop the sections guarded by #if FF_API* > These macros are private and should not be used by external callers. > --- > fftools/ffprobe.c | 34 -- > .../ref/fate/concat-demuxer-extended-lavf-mxf | 2 +- > .../fate/concat-demuxer-extended-lavf-mxf_d10 | 2 +- > .../ref/fate/concat-demuxer-simple1-lavf-mxf | 248 +- > .../fate/concat-demuxer-simple1-lavf-mxf_d10 | 144 +++--- > tests/ref/fate/concat-demuxer-simple2-lavf-ts | 302 ++-- > tests/ref/fate/ffprobe_compact| 34 +- > tests/ref/fate/ffprobe_csv| 34 +- > tests/ref/fate/ffprobe_default| 42 -- > tests/ref/fate/ffprobe_flat | 42 -- > tests/ref/fate/ffprobe_ini| 42 -- > tests/ref/fate/ffprobe_json | 9 - > tests/ref/fate/ffprobe_xml| 6 +- > tests/ref/fate/flcl1905 | 34 +- > ...hapqa-extract-nosnappy-to-hapalphaonly-mov | 8 - > .../fate/hapqa-extract-nosnappy-to-hapq-mov | 8 - > tests/ref/fate/hls-fmp4_ac3 | 1 - > tests/ref/fate/mov-aac-2048-priming | 434 +- > tests/ref/fate/mov-init-nonkeyframe | 240 +- > tests/ref/fate/mov-zombie | 134 +++--- > tests/ref/fate/mxf-probe-applehdr10 | 10 - > tests/ref/fate/mxf-probe-d10 | 8 - > tests/ref/fate/mxf-probe-dnxhd| 9 - > tests/ref/fate/mxf-probe-dv25 | 10 - > 24 files changed, 807 insertions(+), 1030 deletions(-) This decreases the amount of fields regression tested. Iam not sure if thats a good idea. Generally more testing is better [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
Yeah, I am looking into the test failure now. Sorry about that. I did run "make fate" prior to submitting the patch, but I missed the fact that I needed the sample files first, so it looked like it passed all the tests at the time. I have the sample files now, and I am correctly seeing the test failures locally. The test failures did point out a bug in the patch, and I am fixing it now. I will submit an updated patch as soon as that is done. Thanks, Levi On Wed, Jan 27, 2021 at 3:19 PM Andreas Rheinhardt < andreas.rheinha...@gmail.com> wrote: > Aman Karmani: > > On Mon, Jan 25, 2021 at 3:16 PM Levi Dooley > > wrote: > > > >> There was an assumption in the existing code that indentation would not > >> occur more than once on the same row. > >> This was a bad assumption. There are examples of 608 streams which call > >> handle_pac multiple times on the same row with different indentation. > >> As the code was before this change, the new indentation would overwrite > >> existing text with spaces. > >> These changes make indentation skip over columns instead. Text gets > cleared > >> with spaces on handle_edm. > >> Instead of relying on the null character, trailing spaces are trimmed > off > >> the end of a row. > >> This is necessary so that a null character doesn't end up between two > >> words. > >> > >> Signed-off-by: Levi Dooley > >> > >> Here's a link to a sample file that will reproduce this issue. > >> > >> > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip.ts > >> > >> The issue can be reproduced by running the following command: > >> > >>> ffmpeg -f lavfi -i "movie=cleveland-clip.ts[out0+subcc]" -map s > >>> cleveland-clip.ass > >> > >> > >> I've gone ahead and ran this command both before and after my code > changes. > >> The following output files demonstrate that there are some clear cases > of > >> missing words or sentences in the beforepatch file, and it is entirely > >> fixed by this patch in the afterpatch file. > >> > >> Before this patch: > >> > >> > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-beforepatch.ass > >> > >> After this patch: > >> > >> > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-afterpatch.ass > >> > >> And here is the full sample video in case anyone wants to play around > with > >> a larger example with many more caption errors. The above video sample > >> "cleveland-clip.ts" is just a 60 second clip of the following. > >> > >> > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/The%20Cleveland%20Show%20-%20%28Brown%20Magic%29-2018-12-14-0.ts > >> > >> The full patch file is attached to this email. > > > > > > Patch looks reasonable to me. Thanks for sharing the sample and > > before/after output. > > > > Will commit in a couple days if there are no additional comments. > > > > Aman > > > This patch breaks FATE [1]; this needs to be analyzed. If the changes > are ok, the ref files need to be updated in the same patch. > > - Andreas > > [1]: > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210126215003.178259-1-l...@snapstream.com/ > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxfenc: add Coding Equations and Color Primaries to local tags
ons 2021-01-27 klockan 22:07 +0100 skrev Marton Balint: > Fixes ticket #9079. > > Signed-off-by: Marton Balint > --- > libavformat/mxfenc.c| 2 ++ > tests/ref/fate/copy-trac4914| 2 +- > tests/ref/fate/mxf-d10-user-comments| 2 +- > 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, 16 insertions(+), 14 deletions(-) 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] avformat/mxf: Establish register of local tags
ons 2021-01-27 klockan 22:24 +0100 skrev Tomas Härdin:
> ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint:
> > On Wed, 27 Jan 2021, Tomas Härdin wrote:
> >
> > > Hi
> > >
> > > Ticket #9079 brought this about. This should prevent accidentally
> > > adding local tags that are not registered in the primer. It also allows
> > > us to omit tags that we know won't be used, in a manner that is more
> > > elegant than the old code.
> > >
> > > The actual meat of this patch is mxf_mark_tag_unused(),
> > > mxf_write_primer_pack(), mxf_write_local_tag() and
> > > ff_mxf_lookup_local_tag()
> >
> > IMHO you should not move the local tags to mxf.c, because only encoding
> > uses them.
> >
> > The only exception where sharing made sense is
> > ff_mxf_mastering_display_local_tags, but that is super ugly that you
> > now lookup them in mxfdec.c based on local tags we assign them for
> > encoding. Not to mention the linear search you use for each lookup...
>
> We could sort them and use a binary search, but I wanted some feedback
> on this idea before going further. There's not terribly many of them
>
> I'd like to avoid having the full ULs twice in the code. The only way I
> can see how to do that is with #defines
>
> > So I suggest you simply duplicate the 4 UL-s to the single local tags
> > array you make and keep them in mxfenc.c, that way you also don't have to
> > specify the array size manually...
>
> That might conflict with Andreas' deduplication efforts. But yeah, the
> thought did occur to me
Here's an updated patch. Feedback welcome.
/Tomas
From a4d1b438b9f1d78e595ef7795c25a37651bb80bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?=
Date: Wed, 27 Jan 2021 14:08:55 +0100
Subject: [PATCH] avformat/mxf: Establish register of local tags
Tags can be marked "not used" upfront, saving some space in the primer.
av_asserts0() is used to enforce that only tags that are in the primer can actually be written.
Sharing of MasteringDisplay ULs is now done via macros.
---
libavformat/mxf.c| 11 --
libavformat/mxf.h| 8 +-
libavformat/mxfdec.c | 18 +-
libavformat/mxfenc.c | 429 +++
4 files changed, 250 insertions(+), 216 deletions(-)
diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 1901b24c68..85a65f8718 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -22,19 +22,8 @@
#include "libavutil/common.h"
#include "mxf.h"
-const uint8_t ff_mxf_mastering_display_prefix[13] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01 };
-
const uint8_t ff_mxf_random_index_pack_key[16] = { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00 };
-/* be careful to update references to this array if reordering */
-/* local tags are dynamic and must not clash with others in mxfenc.c */
-const MXFLocalTagPair ff_mxf_mastering_display_local_tags[4] = {
-{ 0x8301, { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x01,0x00,0x00 }}, /* Mastering Display Primaries */
-{ 0x8302, { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x02,0x00,0x00 }}, /* Mastering Display White Point Chromaticity */
-{ 0x8303, { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x03,0x00,0x00 }}, /* Mastering Display Maximum Luminance */
-{ 0x8304, { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01,0x04,0x00,0x00 }} /* Mastering Display Minimum Luminance */
-};
-
/**
* SMPTE RP224 http://www.smpte-ra.org/mdd/index.html
*/
diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index 5219abc767..b1b1fedac7 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -83,9 +83,13 @@ typedef struct MXFLocalTagPair {
UID uid;
} MXFLocalTagPair;
-extern const uint8_t ff_mxf_mastering_display_prefix[13];
extern const uint8_t ff_mxf_random_index_pack_key[16];
-extern const MXFLocalTagPair ff_mxf_mastering_display_local_tags[4];
+
+#define FF_MXF_MasteringDisplay_PREFIX 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x04,0x20,0x04,0x01,0x01
+#define FF_MXF_MasteringDisplayPrimaries{ FF_MXF_MasteringDisplay_PREFIX,0x01,0x00,0x00 }
+#define FF_MXF_MasteringDisplayWhitePointChromaticity { FF_MXF_MasteringDisplay_PREFIX,0x02,0x00,0x00 }
+#define FF_MXF_MasteringDisplayMaximumLuminance { FF_MXF_MasteringDisplay_PREFIX,0x03,0x00,0x00 }
+#define FF_MXF_MasteringDisplayMinimumLuminance { FF_MXF_MasteringDisplay_PREFIX,0x04,0x00,0x00 }
#define FF_MXF_MASTERING_CHROMA_DEN 5
#define FF_MXF_MASTERING_LUMA_DEN 1
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index afff20402d..93fa7a97c0 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -336,6 +336,14 @@ static const uint8_t mxf_indirect_value_utf16be[] = { 0x42,0x01,0x10,0x
static const uint8_t mxf_apple_coll_max_cll[] = { 0x06,0x0e,0x2b,0x34,0x01,0x01,0x01,0x0e,0x0e,0x20,0x04,0x01,0x05,0x
Re: [FFmpeg-devel] [PATCH] avdevice/avdevice: Deprecate AVDevice Capabilities API
On 27/01/2021 14:36, Nicolas George wrote: Mark Thompson (12021-01-26): Even after such a merge, the libavdevice API is still a problem. I will re-center the discussion on this alone. And I ask, simply: what problem exactly? See for example the list of suggestions I made for improving the API a few days ago. On 20/01/2021 12:41, Mark Thompson wrote: > * Handle frames as well as packets. >* Including hardware frames - DRM objects from KMS/V4L2, D3D surfaces from Windows desktop duplication (which doesn't currently exist but should). > * Clear core option set - currently almost everything is set by inconsistent private options; things like pixel/sample format, frame/sample rate, geometry and hardware device should be common options to all. > * Asynchronicity - a big annoyance in current recording scenarios with the ffmpeg utility is that both audio and video capture block, and do so on the same thread which results in skipped frames. > * Capability probing - the existing method of options which log the capabilities are not very useful for API users. You, who AFAIK, do not maintain anything in libavdevice and have never used it in a project, say there is a problem with its API. I, who maintain parts of libavdevice and have been using it in projects for years, say there is no problem, there are ways to enhance it, but on the whole it works well enough. Who might be right? Maybe the person with the most experience, don't you think? How are we to determine who has the most relevant experience, so that we avoid appealing to a false authority? For example, we could ask who has made the most changes to the library in the last few years: $ git log --since 2015 libavdevice/ | grep "^Author:" | grep 'Nicolas\|Mark' | sort | uniq -c | sort -n 9 Author: Mark Thompson 7 Author: Nicolas George Or maybe we could go by lines of code? $ git log --since 2015 --author='s...@jkqxz.net' --patch libavdevice/ | diffstat -s 9 files changed, 827 insertions(+), 121 deletions(-) $ git log --since 2015 --author='geo...@nsup.org' --patch libavdevice/ | diffstat -s 6 files changed, 73 insertions(+), 61 deletions(-) Or maybe these are completely meaningless measures which can be cherry-picked to show whatever answer we want and we should instead consider the merits of the proposals involved rather than trying to dismiss the person making them? Therefore, I will cut short this discussion: If you want to know what my ideas are to enhance the API of libavdevice, just ask, and if you'll read the answer I'll be happy to explain at lengths. Please will you explain what your ideas are about how to enhance the API of libavdevice. Even if we disagree about exactly where such changes should be implemented, I would very much welcome hearing about the improvements you would like to make underneath. If you have a diagnostic of an actual problem, please share it. If you have a detailed plan to make libavdevice's API significantly better, then please explain it, I am keenly interested. Another point: remember that in this project, we have a policy of no new API without user: we do not add a new function or API just for the fun of it; we add a new function or API when and if it allows to add a feature, to simplify existing code, etc. So, if you invent a new design for devices, and you implement an useful new feature thanks to this new design, or even better, several useful new features, then excellent. Indeed. The most obvious use-case for much of what I have said is of course the ffmpeg utility itself - the ability to avoid pointless copies when working with devices and to be able to record video and audio at the same time without weird interactions would both be useful features. I should note that my original intent in engaging with this discussion was to gather thoughts from other members of the project wrt this sort of improvement before doing significant work on it, to avoid proceeding down a path which wouldn't go anywhere useful. But if you propose to just add a grand new design for devices, and add a wrapper so that existing devices can be used unchanged, and another wrapper so that existing applications can still use the old API, and that's it, then you are making it worse. So, since AFAIK, you do not have plans to add new useful features to devices, or to libavfilter as a whole, my advice is: for now, leave it alone, until you have given it much more thought. (In return, I will continue to refrain from criticizing the design of the VAAPI stuff.) I would prefer that you do not refrain from offering constructive criticism of "the VAAPI stuff", or anything else that I work on, should you have any. Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mxf: Establish register of local tags
ons 2021-01-27 klockan 21:38 +0100 skrev Marton Balint: > > On Wed, 27 Jan 2021, Tomas Härdin wrote: > > > Hi > > > > Ticket #9079 brought this about. This should prevent accidentally > > adding local tags that are not registered in the primer. It also allows > > us to omit tags that we know won't be used, in a manner that is more > > elegant than the old code. > > > > The actual meat of this patch is mxf_mark_tag_unused(), > > mxf_write_primer_pack(), mxf_write_local_tag() and > > ff_mxf_lookup_local_tag() > > IMHO you should not move the local tags to mxf.c, because only encoding > uses them. > > The only exception where sharing made sense is > ff_mxf_mastering_display_local_tags, but that is super ugly that you > now lookup them in mxfdec.c based on local tags we assign them for > encoding. Not to mention the linear search you use for each lookup... We could sort them and use a binary search, but I wanted some feedback on this idea before going further. There's not terribly many of them I'd like to avoid having the full ULs twice in the code. The only way I can see how to do that is with #defines > So I suggest you simply duplicate the 4 UL-s to the single local tags > array you make and keep them in mxfenc.c, that way you also don't have to > specify the array size manually... That might conflict with Andreas' deduplication efforts. But yeah, the thought did occur 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] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
Aman Karmani: > On Mon, Jan 25, 2021 at 3:16 PM Levi Dooley > wrote: > >> There was an assumption in the existing code that indentation would not >> occur more than once on the same row. >> This was a bad assumption. There are examples of 608 streams which call >> handle_pac multiple times on the same row with different indentation. >> As the code was before this change, the new indentation would overwrite >> existing text with spaces. >> These changes make indentation skip over columns instead. Text gets cleared >> with spaces on handle_edm. >> Instead of relying on the null character, trailing spaces are trimmed off >> the end of a row. >> This is necessary so that a null character doesn't end up between two >> words. >> >> Signed-off-by: Levi Dooley >> >> Here's a link to a sample file that will reproduce this issue. >> >> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip.ts >> >> The issue can be reproduced by running the following command: >> >>> ffmpeg -f lavfi -i "movie=cleveland-clip.ts[out0+subcc]" -map s >>> cleveland-clip.ass >> >> >> I've gone ahead and ran this command both before and after my code changes. >> The following output files demonstrate that there are some clear cases of >> missing words or sentences in the beforepatch file, and it is entirely >> fixed by this patch in the afterpatch file. >> >> Before this patch: >> >> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-beforepatch.ass >> >> After this patch: >> >> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-afterpatch.ass >> >> And here is the full sample video in case anyone wants to play around with >> a larger example with many more caption errors. The above video sample >> "cleveland-clip.ts" is just a 60 second clip of the following. >> >> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/The%20Cleveland%20Show%20-%20%28Brown%20Magic%29-2018-12-14-0.ts >> >> The full patch file is attached to this email. > > > Patch looks reasonable to me. Thanks for sharing the sample and > before/after output. > > Will commit in a couple days if there are no additional comments. > > Aman > This patch breaks FATE [1]; this needs to be analyzed. If the changes are ok, the ref files need to be updated in the same patch. - Andreas [1]: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210126215003.178259-1-l...@snapstream.com/ ___ 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] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.
On Mon, Jan 25, 2021 at 3:16 PM Levi Dooley wrote: > There was an assumption in the existing code that indentation would not > occur more than once on the same row. > This was a bad assumption. There are examples of 608 streams which call > handle_pac multiple times on the same row with different indentation. > As the code was before this change, the new indentation would overwrite > existing text with spaces. > These changes make indentation skip over columns instead. Text gets cleared > with spaces on handle_edm. > Instead of relying on the null character, trailing spaces are trimmed off > the end of a row. > This is necessary so that a null character doesn't end up between two > words. > > Signed-off-by: Levi Dooley > > Here's a link to a sample file that will reproduce this issue. > > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip.ts > > The issue can be reproduced by running the following command: > > > ffmpeg -f lavfi -i "movie=cleveland-clip.ts[out0+subcc]" -map s > > cleveland-clip.ass > > > I've gone ahead and ran this command both before and after my code changes. > The following output files demonstrate that there are some clear cases of > missing words or sentences in the beforepatch file, and it is entirely > fixed by this patch in the afterpatch file. > > Before this patch: > > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-beforepatch.ass > > After this patch: > > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-afterpatch.ass > > And here is the full sample video in case anyone wants to play around with > a larger example with many more caption errors. The above video sample > "cleveland-clip.ts" is just a 60 second clip of the following. > > https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/The%20Cleveland%20Show%20-%20%28Brown%20Magic%29-2018-12-14-0.ts > > The full patch file is attached to this email. Patch looks reasonable to me. Thanks for sharing the sample and before/after output. Will commit in a couple days if there are no additional comments. Aman > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mxfenc: add Coding Equations and Color Primaries to local tags
Fixes ticket #9079.
Signed-off-by: Marton Balint
---
libavformat/mxfenc.c| 2 ++
tests/ref/fate/copy-trac4914| 2 +-
tests/ref/fate/mxf-d10-user-comments| 2 +-
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, 16 insertions(+), 14 deletions(-)
diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 6c5331ad62..0c464d4cfd 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -361,6 +361,8 @@ static const MXFLocalTagPair mxf_local_tag_batch[] = {
{ 0x3217,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x04,0x01,0x03,0x02,0x07,0x00,0x00,0x00}},
/* Display F2 offset */
{ 0x320E,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x04,0x01,0x01,0x01,0x01,0x00,0x00,0x00}},
/* Aspect Ratio */
{ 0x3210,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x02,0x01,0x01,0x01,0x02,0x00}},
/* Transfer characteristic */
+{ 0x321A,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x02,0x01,0x01,0x03,0x01,0x00}},
/* Coding Equations (color space) */
+{ 0x3219,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x09,0x04,0x01,0x02,0x01,0x01,0x06,0x01,0x00}},
/* Color Primaries */
{ 0x3213,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x18,0x01,0x02,0x00,0x00,0x00,0x00}},
/* Image Start Offset */
{ 0x3214,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x18,0x01,0x03,0x00,0x00,0x00,0x00}},
/* Image End Offset */
{ 0x3201,
{0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x04,0x01,0x06,0x01,0x00,0x00,0x00,0x00}},
/* Picture Essence Coding */
diff --git a/tests/ref/fate/copy-trac4914 b/tests/ref/fate/copy-trac4914
index 7c8d0e9158..e8f8afb467 100644
--- a/tests/ref/fate/copy-trac4914
+++ b/tests/ref/fate/copy-trac4914
@@ -1,4 +1,4 @@
-b37c4d5693cdb5b9ed9b33501ffb682a *tests/data/fate/copy-trac4914.mxf
+5d58a32f21b78169d925845f783054e6 *tests/data/fate/copy-trac4914.mxf
561721 tests/data/fate/copy-trac4914.mxf
#tb 0: 1001/3
#media_type 0: video
diff --git a/tests/ref/fate/mxf-d10-user-comments
b/tests/ref/fate/mxf-d10-user-comments
index de4f26c6f8..3b9d9d2142 100644
--- a/tests/ref/fate/mxf-d10-user-comments
+++ b/tests/ref/fate/mxf-d10-user-comments
@@ -1 +1 @@
-68f0fa62b6a676894afbbe4c34ebf70b
+fe9b43f5b6e7737fe2670b660fd3d860
diff --git a/tests/ref/fate/mxf-opatom-user-comments
b/tests/ref/fate/mxf-opatom-user-comments
index 90e3fb229a..be57eb4e19 100644
--- a/tests/ref/fate/mxf-opatom-user-comments
+++ b/tests/ref/fate/mxf-opatom-user-comments
@@ -1 +1 @@
-f6760a9e710ba478bc3949f3e5c9b34a
+9b3d7201c37c5783702774e46e0da141
diff --git a/tests/ref/fate/mxf-reel_name b/tests/ref/fate/mxf-reel_name
index 16022b1789..acda8fe43a 100644
--- a/tests/ref/fate/mxf-reel_name
+++ b/tests/ref/fate/mxf-reel_name
@@ -1 +1 @@
-73a891041b2fc836a893ffb49fff4fff
+422d6ed4821e7bbecbcdecc553abc6e4
diff --git a/tests/ref/fate/mxf-user-comments b/tests/ref/fate/mxf-user-comments
index ddf51d939c..3b77db9118 100644
--- a/tests/ref/fate/mxf-user-comments
+++ b/tests/ref/fate/mxf-user-comments
@@ -1 +1 @@
-1255faf854223a74d707553121e5eca3
+c77b632dbcdacd6466e1ec794917556e
diff --git a/tests/ref/fate/time_base b/tests/ref/fate/time_base
index 710fde1002..d3c97956d6 100644
--- a/tests/ref/fate/time_base
+++ b/tests/ref/fate/time_base
@@ -1 +1 @@
-42863a53f6c63efbc8c5a2eb76f13f5f
+e5c9da6972b6f6e7b15bcbbf20a9dbd1
diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf
index 5b16496f06..1b1fe1a734 100644
--- a/tests/ref/lavf/mxf
+++ b/tests/ref/lavf/mxf
@@ -1,9 +1,9 @@
-27b98795036b334e100c15c7e06d948f *tests/data/lavf/lavf.mxf
+1703da2e9f0ca49a5f0bcbcc6944c9c0 *tests/data/lavf/lavf.mxf
526393 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0x8dddfaab
-783b475a818602f54e947094d57e2981 *tests/data/lavf/lavf.mxf
+b44c4f138f1a87256211c3112dd52c87 *tests/data/lavf/lavf.mxf
561721 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0x96ff1b48
-02bf8f0cd8951a49e277306691cb1538 *tests/data/lavf/lavf.mxf
+9ede3c95054c30eb815fbe598a1f097b *tests/data/lavf/lavf.mxf
526393 tests/data/lavf/lavf.mxf
tests/data/lavf/lavf.mxf CRC=0x8dddfaab
diff --git a/tests/ref/lavf/mxf_d10 b/tests/ref/lavf/mxf_d10
index 30701619e0..19e6f20b51 100644
--- a/tests/ref/lavf/mxf_d10
+++ b/tests/ref/lavf/mxf_d10
@@ -1,3 +1,3 @@
-da0ebbebb50a530b14c0f06017f464b3 *tests/data/lavf/lavf.mxf_d10
+c72f1f3d6ce555f96946c421f705f880 *tests/data/lavf/lavf.mxf_d10
5332013 tests/data/lavf/lavf.mxf_d10
tests/data/lavf/lavf.mxf_d10 CRC=0x6c74d488
diff --git a/tests/ref/lavf/mxf_dv25 b/tests/ref/lavf/mxf_dv25
index d4559df862..2951538cae 100644
--- a/tests/ref/
Re: [FFmpeg-devel] [PATCH] avcodec/flashsv2enc: Fix use of uninitialized value
On Wed, 27 Jan 2021, Andreas Rheinhardt wrote:
Before 257a83b969157eb76c18158a4e503e908d8b1125, certain buffers were
zero-allocated in the init function and only reallocated lateron if they
turned out to be too small; now they are only allocated during init,
leading to use-of-uninitialized values lateron. The same could happen
before if the dimensions are big enough so that the buffers would be
reallocated, as the new part of the reallocated buffer would not be
zeroed (happened for 960x960). So always zero the buffers in the
function designed to init them.
LGTM, thanks.
Signed-off-by: Andreas Rheinhardt
---
If no one objects, I'll send a patch to remove #ifndef FLASHSV2_DUMB stuff
lateron: It doesn't even compile any more and given that it has never
worked it stands to reason that any successfull non-dumb way needs to be
different from the currently outcommented code. Of course, I don't think
that anyone will ever add a successfull non-dumb way for this encoder
for an old format.
Good idea, i thought about it too.
Furthermore, there are more bugs lurking in this code, namely the
ptr = av_realloc_array(ptr, size) which leads to memleaks on
reallocation failures as well as problems if the caller tries to call
the encoder lateron because block_width/height have already been set, so
that no reallocation attempt would be performed.
Yes, indeed.
Thanks for taking care of these.
Regards,
Marton
libavcodec/flashsv2enc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavcodec/flashsv2enc.c b/libavcodec/flashsv2enc.c
index 5139b17a28..430b6806c8 100644
--- a/libavcodec/flashsv2enc.c
+++ b/libavcodec/flashsv2enc.c
@@ -142,6 +142,7 @@ static void init_blocks(FlashSV2Context * s, Block * blocks,
{
int row, col;
Block *b;
+memset(blocks, 0, s->cols * s->rows * sizeof(*blocks));
for (col = 0; col < s->cols; col++) {
for (row = 0; row < s->rows; row++) {
b = blocks + (col + row * s->cols);
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel 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: Establish register of local tags
On Wed, 27 Jan 2021, Tomas Härdin wrote: Hi Ticket #9079 brought this about. This should prevent accidentally adding local tags that are not registered in the primer. It also allows us to omit tags that we know won't be used, in a manner that is more elegant than the old code. The actual meat of this patch is mxf_mark_tag_unused(), mxf_write_primer_pack(), mxf_write_local_tag() and ff_mxf_lookup_local_tag() IMHO you should not move the local tags to mxf.c, because only encoding uses them. The only exception where sharing made sense is ff_mxf_mastering_display_local_tags, but that is super ugly that you now lookup them in mxfdec.c based on local tags we assign them for encoding. Not to mention the linear search you use for each lookup... So I suggest you simply duplicate the 4 UL-s to the single local tags array you make and keep them in mxfenc.c, that way you also don't have to specify the array size manually... Regards, Marton ___ 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] avpacket: RFC for ABI bump additions
On 1/27/2021 2:40 PM, Lynne wrote:
Jan 27, 2021, 16:07 by jamr...@gmail.com:
On 1/27/2021 6:16 AM, Anton Khirnov wrote:
Quoting James Almer (2021-01-26 20:11:16)
On 1/26/2021 1:17 PM, Anton Khirnov wrote:
We could start by adding a field to AVPacket that would be set to a
magic value by av_packet_alloc().
Then have e.g. AVCodecContext/AVFormatContext warn when they see a
packet without this magic value.
I don't like much the idea of adding a public field just to emit a
deprecation warning.
int internal_do_not_touch; // do not touch
is not really public. It is visible in the public headers, but so are
all the AVFooInternal. I agree that it is not the prettiest thing ever,
but it's not too bad.
And I believe it would solve a real problem, since we have few other
ways to let our users know they need to change something. Most of them
do not follow development closely, I'd think.
If sizeof(AVPacket) stops being part of the ABI, then av_init_packet() becomes
unnecessary, right? av_packet_unref() will be the only valid way for the user
to reuse the AVPacket. So that deprecation warning might be enough for stack
users.
I think just a compile-time deprecation on av_init_packet() would be enough.
Regarding a new internal field that lavf could check, i don't think it's enough
since it may be uninitialized for packets on stack. And you can't make
av_init_packet() set it to 0/NULL because then av_packet_unref() will also
reset it, and lavf will start printing bogus warnings for allocated packet
users.
I'd really rather not spam the log of users with API deprecation messages. We're
already guilty enough of doing that with color_range.
Ok, something like this patch, then?
Assuming we go through with it, I don't know what would be better, if to
push it now for ffmpeg 4.4 (Which should be branched out before the
bump), or after the bump.
The former would give a lot more time for downstreams to adapt, since it
will be in a release for the majority of the deprecation period rather
than exclusively on the master branch until the first post-bump release.
There's also a lot of stack usage for AVPacket within libav*, so we will
have to either remove them all alongside the deprecation, or at least
silence the av_init_packet() warnings while we slowly go through all of
them.
From 75d82c56aca9d1905865e0b1e40258ec0da61e6b Mon Sep 17 00:00:00 2001
From: James Almer
Date: Wed, 27 Jan 2021 16:24:10 -0300
Subject: [PATCH] avcodec/packet: deprecate av_init_packet()
Once removed, sizeof(AVPacket) will stop being a part of the public ABI.
Signed-off-by: James Almer
---
libavcodec/avpacket.c | 34 ++
libavcodec/packet.h | 19 +++
libavcodec/version.h | 3 +++
3 files changed, 44 insertions(+), 12 deletions(-)
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index e4ba403cf6..e6cae39c81 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -32,6 +32,7 @@
#include "packet.h"
#include "packet_internal.h"
+#if FF_API_INIT_PACKET
void av_init_packet(AVPacket *pkt)
{
pkt->pts = AV_NOPTS_VALUE;
@@ -49,6 +50,27 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->side_data= NULL;
pkt->side_data_elems = 0;
}
+#endif
+
+static void packet_init(AVPacket *pkt)
+{
+pkt->pts = AV_NOPTS_VALUE;
+pkt->dts = AV_NOPTS_VALUE;
+pkt->pos = -1;
+pkt->duration= 0;
+#if FF_API_CONVERGENCE_DURATION
+FF_DISABLE_DEPRECATION_WARNINGS
+pkt->convergence_duration = 0;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
+pkt->flags = 0;
+pkt->stream_index= 0;
+pkt->buf = NULL;
+pkt->data= NULL;
+pkt->side_data = NULL;
+pkt->side_data_elems = 0;
+pkt->size= 0;
+}
AVPacket *av_packet_alloc(void)
{
@@ -56,7 +78,7 @@ AVPacket *av_packet_alloc(void)
if (!pkt)
return pkt;
-av_init_packet(pkt);
+packet_init(pkt);
return pkt;
}
@@ -92,7 +114,7 @@ int av_new_packet(AVPacket *pkt, int size)
if (ret < 0)
return ret;
-av_init_packet(pkt);
+packet_init(pkt);
pkt->buf = buf;
pkt->data = buf->data;
pkt->size = size;
@@ -607,9 +629,7 @@ void av_packet_unref(AVPacket *pkt)
{
av_packet_free_side_data(pkt);
av_buffer_unref(&pkt->buf);
-av_init_packet(pkt);
-pkt->data = NULL;
-pkt->size = 0;
+packet_init(pkt);
}
int av_packet_ref(AVPacket *dst, const AVPacket *src)
@@ -664,9 +684,7 @@ AVPacket *av_packet_clone(const AVPacket *src)
void av_packet_move_ref(AVPacket *dst, AVPacket *src)
{
*dst = *src;
-av_init_packet(src);
-src->data = NULL;
-src->size = 0;
+packet_init(src);
}
int av_packet_make_refcounted(AVPacket *pkt)
diff --git a/libavcodec/packet.h b/libavcodec/packet.h
index b9d4c9c2c8..d7f34d109e 100644
--- a/libavcodec/packet.h
+++ b/libavc
[FFmpeg-devel] [PATCH 2/2] avfilter/vf_lut3d: add prism interpolation
Signed-off-by: Paul B Mahol
---
doc/filters.texi | 2 ++
libavfilter/vf_lut3d.c | 57 ++
2 files changed, 59 insertions(+)
diff --git a/doc/filters.texi b/doc/filters.texi
index a38f9b4124..52458c116d 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -13868,6 +13868,8 @@ Interpolate values using the 8 points defining a cube.
Interpolate values using a tetrahedron.
@item pyramid
Interpolate values using a pyramid.
+@item prism
+Interpolate values using a prism.
@end table
@end table
diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
index 12eb15fe67..fb33c93c34 100644
--- a/libavfilter/vf_lut3d.c
+++ b/libavfilter/vf_lut3d.c
@@ -50,6 +50,7 @@ enum interp_mode {
INTERPOLATE_TRILINEAR,
INTERPOLATE_TETRAHEDRAL,
INTERPOLATE_PYRAMID,
+INTERPOLATE_PRISM,
NB_INTERP_MODE
};
@@ -105,6 +106,7 @@ typedef struct ThreadData {
{ "trilinear", "interpolate values using the 8 points defining a
cube", 0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_TRILINEAR}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ "tetrahedral", "interpolate values using a tetrahedron",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_TETRAHEDRAL}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ "pyramid", "interpolate values using a pyramid",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_PYRAMID}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
+{ "prism", "interpolate values using a prism",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_PRISM}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ NULL }
#define EXPONENT_MASK 0x7F80
@@ -237,6 +239,51 @@ static inline struct rgbvec interp_pyramid(const
LUT3DContext *lut3d,
return c;
}
+static inline struct rgbvec interp_prism(const LUT3DContext *lut3d,
+ const struct rgbvec *s)
+{
+const int lutsize2 = lut3d->lutsize2;
+const int lutsize = lut3d->lutsize;
+const int prev[] = {PREV(s->r), PREV(s->g), PREV(s->b)};
+const int next[] = {NEXT(s->r), NEXT(s->g), NEXT(s->b)};
+const struct rgbvec d = {s->r - prev[0], s->g - prev[1], s->b - prev[2]};
+const struct rgbvec c000 = lut3d->lut[prev[0] * lutsize2 + prev[1] *
lutsize + prev[2]];
+const struct rgbvec c010 = lut3d->lut[prev[0] * lutsize2 + next[1] *
lutsize + prev[2]];
+const struct rgbvec c101 = lut3d->lut[next[0] * lutsize2 + prev[1] *
lutsize + next[2]];
+const struct rgbvec c111 = lut3d->lut[next[0] * lutsize2 + next[1] *
lutsize + next[2]];
+struct rgbvec c;
+
+if (d.b > d.r) {
+const struct rgbvec c001 = lut3d->lut[prev[0] * lutsize2 + prev[1] *
lutsize + next[2]];
+const struct rgbvec c011 = lut3d->lut[prev[0] * lutsize2 + next[1] *
lutsize + next[2]];
+
+c.r = c000.r + (c001.r - c000.r) * d.b + (c101.r - c001.r) * d.r +
(c010.r - c000.r) * d.g +
+ (c000.r - c010.r - c001.r + c011.r) * d.b * d.g +
+ (c001.r - c011.r - c101.r + c111.r) * d.r * d.g;
+c.g = c000.g + (c001.g - c000.g) * d.b + (c101.g - c001.g) * d.r +
(c010.g - c000.g) * d.g +
+ (c000.g - c010.g - c001.g + c011.g) * d.b * d.g +
+ (c001.g - c011.g - c101.g + c111.g) * d.r * d.g;
+c.b = c000.b + (c001.b - c000.b) * d.b + (c101.b - c001.b) * d.r +
(c010.b - c000.b) * d.g +
+ (c000.b - c010.b - c001.b + c011.b) * d.b * d.g +
+ (c001.b - c011.b - c101.b + c111.b) * d.r * d.g;
+} else {
+const struct rgbvec c110 = lut3d->lut[next[0] * lutsize2 + next[1] *
lutsize + prev[2]];
+const struct rgbvec c100 = lut3d->lut[next[0] * lutsize2 + prev[1] *
lutsize + prev[2]];
+
+c.r = c000.r + (c101.r - c100.r) * d.b + (c100.r - c000.r) * d.r +
(c010.r - c000.r) * d.g +
+ (c100.r - c110.r - c101.r + c111.r) * d.b * d.g +
+ (c000.r - c010.r - c100.r + c110.r) * d.r * d.g;
+c.g = c000.g + (c101.g - c100.g) * d.b + (c100.g - c000.g) * d.r +
(c010.g - c000.g) * d.g +
+ (c100.g - c110.g - c101.g + c111.g) * d.b * d.g +
+ (c000.g - c010.g - c100.g + c110.g) * d.r * d.g;
+c.b = c000.b + (c101.b - c100.b) * d.b + (c100.b - c000.b) * d.r +
(c010.b - c000.b) * d.g +
+ (c100.b - c110.b - c101.b + c111.b) * d.b * d.g +
+ (c000.b - c010.b - c100.b + c110.b) * d.r * d.g;
+}
+
+return c;
+}
+
/**
* Tetrahedral interpolation. Based on code found in Truelight Software
Library paper.
* @see
http://www.filmlight.ltd.uk/pdf/whitepapers/FL-TL-TN-0057-SoftwareLib.pdf
@@ -390,31 +437,37 @@ DEFINE_INTERP_FUNC_PLANAR(nearest, 8, 8)
DEFINE_INTERP_FUNC_PLANAR(trilinear, 8, 8)
DEFINE_INTERP_FUNC_PLANAR(tetrahedral, 8, 8)
DEFINE_INTERP_FUNC_PLANAR(pyramid, 8, 8)
+DEFINE_INTERP_FUNC_PLANAR(prism, 8, 8)
DEFINE_INTERP_FUNC_PLANAR(nearest, 16, 9)
DEFINE_INTERP_F
[FFmpeg-devel] [PATCH 1/2] avfilter/vf_lut3d: add pyramid interpolation
Signed-off-by: Paul B Mahol
---
doc/filters.texi | 2 ++
libavfilter/vf_lut3d.c | 62 ++
2 files changed, 64 insertions(+)
diff --git a/doc/filters.texi b/doc/filters.texi
index fb9995eddf..a38f9b4124 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -13866,6 +13866,8 @@ Use values from the nearest defined point.
Interpolate values using the 8 points defining a cube.
@item tetrahedral
Interpolate values using a tetrahedron.
+@item pyramid
+Interpolate values using a pyramid.
@end table
@end table
diff --git a/libavfilter/vf_lut3d.c b/libavfilter/vf_lut3d.c
index 172d6df0c8..12eb15fe67 100644
--- a/libavfilter/vf_lut3d.c
+++ b/libavfilter/vf_lut3d.c
@@ -49,6 +49,7 @@ enum interp_mode {
INTERPOLATE_NEAREST,
INTERPOLATE_TRILINEAR,
INTERPOLATE_TETRAHEDRAL,
+INTERPOLATE_PYRAMID,
NB_INTERP_MODE
};
@@ -103,6 +104,7 @@ typedef struct ThreadData {
{ "nearest", "use values from the nearest defined points",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_NEAREST}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ "trilinear", "interpolate values using the 8 points defining a
cube", 0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_TRILINEAR}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ "tetrahedral", "interpolate values using a tetrahedron",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_TETRAHEDRAL}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
+{ "pyramid", "interpolate values using a pyramid",
0, AV_OPT_TYPE_CONST, {.i64=INTERPOLATE_PYRAMID}, INT_MIN, INT_MAX,
FLAGS, "interp_mode" }, \
{ NULL }
#define EXPONENT_MASK 0x7F80
@@ -185,6 +187,56 @@ static inline struct rgbvec interp_trilinear(const
LUT3DContext *lut3d,
return c;
}
+static inline struct rgbvec interp_pyramid(const LUT3DContext *lut3d,
+ const struct rgbvec *s)
+{
+const int lutsize2 = lut3d->lutsize2;
+const int lutsize = lut3d->lutsize;
+const int prev[] = {PREV(s->r), PREV(s->g), PREV(s->b)};
+const int next[] = {NEXT(s->r), NEXT(s->g), NEXT(s->b)};
+const struct rgbvec d = {s->r - prev[0], s->g - prev[1], s->b - prev[2]};
+const struct rgbvec c000 = lut3d->lut[prev[0] * lutsize2 + prev[1] *
lutsize + prev[2]];
+const struct rgbvec c111 = lut3d->lut[next[0] * lutsize2 + next[1] *
lutsize + next[2]];
+struct rgbvec c;
+
+if (d.g > d.r && d.b > d.r) {
+const struct rgbvec c001 = lut3d->lut[prev[0] * lutsize2 + prev[1] *
lutsize + next[2]];
+const struct rgbvec c010 = lut3d->lut[prev[0] * lutsize2 + next[1] *
lutsize + prev[2]];
+const struct rgbvec c011 = lut3d->lut[prev[0] * lutsize2 + next[1] *
lutsize + next[2]];
+
+c.r = c000.r + (c111.r - c011.r) * d.r + (c010.r - c000.r) * d.g +
(c001.r - c000.r) * d.b +
+ (c011.r - c001.r - c010.r + c000.r) * d.g * d.b;
+c.g = c000.g + (c111.g - c011.g) * d.r + (c010.g - c000.g) * d.g +
(c001.g - c000.g) * d.b +
+ (c011.g - c001.g - c010.g + c000.g) * d.g * d.b;
+c.b = c000.b + (c111.b - c011.b) * d.r + (c010.b - c000.b) * d.g +
(c001.b - c000.b) * d.b +
+ (c011.b - c001.b - c010.b + c000.b) * d.g * d.b;
+} else if (d.r > d.g && d.b > d.g) {
+const struct rgbvec c001 = lut3d->lut[prev[0] * lutsize2 + prev[1] *
lutsize + next[2]];
+const struct rgbvec c100 = lut3d->lut[next[0] * lutsize2 + prev[1] *
lutsize + prev[2]];
+const struct rgbvec c101 = lut3d->lut[next[0] * lutsize2 + prev[1] *
lutsize + next[2]];
+
+c.r = c000.r + (c100.r - c000.r) * d.r + (c111.r - c101.r) * d.g +
(c001.r - c000.r) * d.b +
+ (c101.r - c001.r - c100.r + c000.r) * d.r * d.b;
+c.g = c000.g + (c100.g - c000.g) * d.r + (c111.g - c101.g) * d.g +
(c001.g - c000.g) * d.b +
+ (c101.g - c001.g - c100.g + c000.g) * d.r * d.b;
+c.b = c000.b + (c100.b - c000.b) * d.r + (c111.b - c101.b) * d.g +
(c001.b - c000.b) * d.b +
+ (c101.b - c001.b - c100.b + c000.b) * d.r * d.b;
+} else {
+const struct rgbvec c010 = lut3d->lut[prev[0] * lutsize2 + next[1] *
lutsize + prev[2]];
+const struct rgbvec c110 = lut3d->lut[next[0] * lutsize2 + next[1] *
lutsize + prev[2]];
+const struct rgbvec c100 = lut3d->lut[next[0] * lutsize2 + prev[1] *
lutsize + prev[2]];
+
+c.r = c000.r + (c100.r - c000.r) * d.r + (c010.r - c000.r) * d.g +
(c111.r - c110.r) * d.b +
+ (c110.r - c100.r - c010.r + c000.r) * d.r * d.g;
+c.g = c000.g + (c100.g - c000.g) * d.r + (c010.g - c000.g) * d.g +
(c111.g - c110.g) * d.b +
+ (c110.g - c100.g - c010.g + c000.g) * d.r * d.g;
+c.b = c000.b + (c100.b - c000.b) * d.r + (c010.b - c000.b) * d.g +
(c111.b - c110.b) * d.b +
+ (c110.b - c100.b - c010.b + c000.b) * d.r * d.g;
+}
+
+return c;
+}
Re: [FFmpeg-devel] [PATCH] avpacket: RFC for ABI bump additions
Jan 27, 2021, 16:07 by jamr...@gmail.com: > On 1/27/2021 6:16 AM, Anton Khirnov wrote: > >> Quoting James Almer (2021-01-26 20:11:16) >> >>> On 1/26/2021 1:17 PM, Anton Khirnov wrote: >>> We could start by adding a field to AVPacket that would be set to a magic value by av_packet_alloc(). Then have e.g. AVCodecContext/AVFormatContext warn when they see a packet without this magic value. >>> >>> I don't like much the idea of adding a public field just to emit a >>> deprecation warning. >>> >> >> >> int internal_do_not_touch; // do not touch >> >> is not really public. It is visible in the public headers, but so are >> all the AVFooInternal. I agree that it is not the prettiest thing ever, >> but it's not too bad. >> >> And I believe it would solve a real problem, since we have few other >> ways to let our users know they need to change something. Most of them >> do not follow development closely, I'd think. >> > > If sizeof(AVPacket) stops being part of the ABI, then av_init_packet() > becomes unnecessary, right? av_packet_unref() will be the only valid way for > the user to reuse the AVPacket. So that deprecation warning might be enough > for stack users. > I think just a compile-time deprecation on av_init_packet() would be enough. > Regarding a new internal field that lavf could check, i don't think it's > enough since it may be uninitialized for packets on stack. And you can't make > av_init_packet() set it to 0/NULL because then av_packet_unref() will also > reset it, and lavf will start printing bogus warnings for allocated packet > users. > I'd really rather not spam the log of users with API deprecation messages. We're already guilty enough of doing that with color_range. ___ 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] avcodec/rl: Improve documentation of ff_rl_init/ff_rl_init_vlc
In particular, document that they initialize different parts of an
RLTable and therefore need not be synchronized.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/rl.h | 15 +++
1 file changed, 15 insertions(+)
diff --git a/libavcodec/rl.h b/libavcodec/rl.h
index 26e0b32a90..5aae698e31 100644
--- a/libavcodec/rl.h
+++ b/libavcodec/rl.h
@@ -49,10 +49,25 @@ typedef struct RLTable {
} RLTable;
/**
+ * Initialize index_run, max_level and max_run from n, last, table_vlc,
+ * table_run and table_level.
* @param static_store static uint8_t array[2][2*MAX_RUN + MAX_LEVEL + 3]
* to hold the level and run tables.
+ * @note This function does not touch rl_vlc at all, hence there is no need
+ *to synchronize calls to ff_rl_init() and ff_rl_init_vlc() using the
+ *same RLTable.
*/
void ff_rl_init(RLTable *rl, uint8_t static_store[2][2*MAX_RUN + MAX_LEVEL +
3]);
+
+/**
+ * Initialize rl_vlc from n, last, table_vlc, table_run and table_level.
+ * All rl_vlc pointers to be initialized must already point to a static
+ * buffer of `static_size` RL_VLC_ELEM elements; if a pointer is NULL,
+ * initializing further VLCs stops.
+ * @note This function does not touch what ff_rl_init() initializes at all,
+ *hence there is no need to synchronize calls to ff_rl_init() and
+ *ff_rl_init_vlc() using the same RLTable.
+ */
void ff_rl_init_vlc(RLTable *rl, unsigned static_size);
#define INIT_VLC_RL(rl, static_size)\
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/3] av_dump_format: add a heading for chapters
Otherwise the chapters look like a part of the metadata section.
---
libavformat/dump.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/dump.c b/libavformat/dump.c
index fe628010d3..ca73c2810c 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -686,6 +686,8 @@ void av_dump_format(AVFormatContext *ic, int index,
av_log(NULL, AV_LOG_INFO, "\n");
}
+if (ic->nb_chapters)
+av_log(NULL, AV_LOG_INFO, " Chapters:\n");
for (i = 0; i < ic->nb_chapters; i++) {
const AVChapter *ch = ic->chapters[i];
av_log(NULL, AV_LOG_INFO, "Chapter #%d:%d: ", index, i);
--
2.28.0
___
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] av_dump_format(): increase indentation for chapter metadata
It should be at a deeper level than the chapter ti belongs to.
---
libavformat/dump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/dump.c b/libavformat/dump.c
index ca73c2810c..5c8e8bc60d 100644
--- a/libavformat/dump.c
+++ b/libavformat/dump.c
@@ -696,7 +696,7 @@ void av_dump_format(AVFormatContext *ic, int index,
av_log(NULL, AV_LOG_INFO,
"end %f\n", ch->end * av_q2d(ch->time_base));
-dump_metadata(NULL, ch->metadata, "");
+dump_metadata(NULL, ch->metadata, " ");
}
if (ic->nb_programs) {
--
2.28.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] av_dump_format(): reduce indentation for streams
Makes it easier to identify where metadata/chapters end and streams begin. --- libavformat/dump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/dump.c b/libavformat/dump.c index 5c8e8bc60d..62ef5e9852 100644 --- a/libavformat/dump.c +++ b/libavformat/dump.c @@ -543,7 +543,7 @@ FF_ENABLE_DEPRECATION_WARNINGS avcodec_string(buf, sizeof(buf), avctx, is_output); avcodec_free_context(&avctx); -av_log(NULL, AV_LOG_INFO, "Stream #%d:%d", index, i); +av_log(NULL, AV_LOG_INFO, " Stream #%d:%d", index, i); /* the pid is an important information, so we display it */ /* XXX: add a generic system */ -- 2.28.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/rtpdec: Avoid allocations of small dynamic buffers
Besides avoiding allocations this also fixes a design defect of
ff_rtp_send_punch_packets: It did not return an error in case of
these allocations failed.
Signed-off-by: Andreas Rheinhardt
---
libavformat/rtpdec.c | 38 +-
1 file changed, 13 insertions(+), 25 deletions(-)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index f40c020c33..d592e34893 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -404,38 +404,26 @@ int ff_rtp_check_and_send_back_rr(RTPDemuxContext *s,
URLContext *fd,
void ff_rtp_send_punch_packets(URLContext *rtp_handle)
{
-AVIOContext *pb;
-uint8_t *buf;
-int len;
+uint8_t buf[RTP_MIN_PACKET_LENGTH], *ptr = buf;
/* Send a small RTP packet */
-if (avio_open_dyn_buf(&pb) < 0)
-return;
-avio_w8(pb, (RTP_VERSION << 6));
-avio_w8(pb, 0); /* Payload type */
-avio_wb16(pb, 0); /* Seq */
-avio_wb32(pb, 0); /* Timestamp */
-avio_wb32(pb, 0); /* SSRC */
+bytestream_put_byte(&ptr, (RTP_VERSION << 6));
+bytestream_put_byte(&ptr, 0); /* Payload type */
+bytestream_put_be16(&ptr, 0); /* Seq */
+bytestream_put_be32(&ptr, 0); /* Timestamp */
+bytestream_put_be32(&ptr, 0); /* SSRC */
-len = avio_close_dyn_buf(pb, &buf);
-if ((len > 0) && buf)
-ffurl_write(rtp_handle, buf, len);
-av_free(buf);
+ffurl_write(rtp_handle, buf, ptr - buf);
/* Send a minimal RTCP RR */
-if (avio_open_dyn_buf(&pb) < 0)
-return;
+ptr = buf;
+bytestream_put_byte(&ptr, (RTP_VERSION << 6));
+bytestream_put_byte(&ptr, RTCP_RR); /* receiver report */
+bytestream_put_be16(&ptr, 1); /* length in words - 1 */
+bytestream_put_be32(&ptr, 0); /* our own SSRC */
-avio_w8(pb, (RTP_VERSION << 6));
-avio_w8(pb, RTCP_RR); /* receiver report */
-avio_wb16(pb, 1); /* length in words - 1 */
-avio_wb32(pb, 0); /* our own SSRC */
-
-len = avio_close_dyn_buf(pb, &buf);
-if ((len > 0) && buf)
-ffurl_write(rtp_handle, buf, len);
-av_free(buf);
+ffurl_write(rtp_handle, buf, ptr - buf);
}
static int find_missing_packets(RTPDemuxContext *s, uint16_t *first_missing,
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/utils: always preserve container dimensions for all streams
On Wed, Jan 27, 2021 at 2:56 PM Anton Khirnov wrote: > > Quoting James Almer (2021-01-26 00:17:51) > > On 1/25/2021 7:46 PM, Michael Niedermayer wrote: > > > On Sun, Jan 24, 2021 at 11:41:13AM -0300, James Almer wrote: > > >> If a decoder is used for probing it may change the dimensions reported > > >> by the > > >> demuxer, either by the lowres factor or because of assorted frames > > >> reporting > > >> different dimensions, and in a codec copy scenario, the last dimensions > > >> arbitrarily set by it could end up being propagated to the muxer. > > >> > > >> Signed-off-by: James Almer > > >> --- > > >> libavformat/utils.c | 10 ++ > > >> tests/ref/fate/cbs-vp9-vp90-2-05-resize | 2 +- > > >> tests/ref/fate/redcode-demux| 2 +- > > >> tests/ref/fate/wtv-demux| 4 ++-- > > >> 4 files changed, 10 insertions(+), 8 deletions(-) > > >> > > > > > > breaks: > > > > > > ./ffmpeg -i tickets/2892/MPEG_tbn_test.mov -c:v copy -c:a copy -vtag > > > mx3n -timecode 10:00:00:00 -vframes 3 file.mov > > > ... > > > [mov @ 0x558785108e00] D-10/IMX must use 720x608 or 720x512 video > > > resolution > > > > So the source mov is faulty and reports a wrong resolution? And trying > > to pass it through instead of letting a decoder change it makes it fail > > to mux because the muxer refuses to create non compliant files. > > That means if there's no decoder in the build, you'd get the same result > > as without this patch. > > > > The mere existence of this code here means that letting decoders set the > > resolution in a codec copy scenario is not ideal, as shown by the fact > > one can tell it to make up an arbitrary resolution like it's the case of > > lowres, or it can just pick up one from an arbitrary frame. Hence > > prioritizing what the demuxer reads from the container feels like the > > correct thing to do. But of course, broken files are a thing. > > > > I don't know, maybe a new AVFMT_FLAG_ flag to choose between giving > > priority to what the container reports or what a decoder sets, if any is > > present, could work around this? > > I'd prefer an ffmpeg.c flag if anything. > > It is not lavf's job to set policy on what information is more > trustworthy. > The demuxer should export what is written in the container, the > decoder/parser should export what is written in the codec. The user then > decides which gets used. > I concur with that approach. User code (including ffmpeg.c) should set a preference if required, avformat should expose container information if present, otherwise information is lost and not recoverable. - Hendrik ___ 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] avpacket: RFC for ABI bump additions
On 1/27/2021 6:16 AM, Anton Khirnov wrote: Quoting James Almer (2021-01-26 20:11:16) On 1/26/2021 1:17 PM, Anton Khirnov wrote: We could start by adding a field to AVPacket that would be set to a magic value by av_packet_alloc(). Then have e.g. AVCodecContext/AVFormatContext warn when they see a packet without this magic value. I don't like much the idea of adding a public field just to emit a deprecation warning. int internal_do_not_touch; // do not touch is not really public. It is visible in the public headers, but so are all the AVFooInternal. I agree that it is not the prettiest thing ever, but it's not too bad. And I believe it would solve a real problem, since we have few other ways to let our users know they need to change something. Most of them do not follow development closely, I'd think. If sizeof(AVPacket) stops being part of the ABI, then av_init_packet() becomes unnecessary, right? av_packet_unref() will be the only valid way for the user to reuse the AVPacket. So that deprecation warning might be enough for stack users. Regarding a new internal field that lavf could check, i don't think it's enough since it may be uninitialized for packets on stack. And you can't make av_init_packet() set it to 0/NULL because then av_packet_unref() will also reset it, and lavf will start printing bogus warnings for allocated packet users. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avdevice/avdevice: Deprecate AVDevice Capabilities API
Jean-Baptiste Kempf (12021-01-26): > Unfortunately, that's not how it works. > We need to accept the will of majority of developers, even if you (or > someone else) is not convinced. We will accept the will of the majority of developers when the time for making a decision comes if no consensus has been reached. But before this point, every single developer in the project should listen to the advice of whoever has the most experience and knowledge about a particular topic. This is an appeal to authority, but the good kind, not "listen to me because I'm famous" but "listen to me because I know more about this particular issue". Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avdevice/avdevice: Deprecate AVDevice Capabilities API
Mark Thompson (12021-01-26): > Even after such a merge, the libavdevice API is still a problem. I will re-center the discussion on this alone. And I ask, simply: what problem exactly? You, who AFAIK, do not maintain anything in libavdevice and have never used it in a project, say there is a problem with its API. I, who maintain parts of libavdevice and have been using it in projects for years, say there is no problem, there are ways to enhance it, but on the whole it works well enough. Who might be right? Maybe the person with the most experience, don't you think? Therefore, I will cut short this discussion: If you want to know what my ideas are to enhance the API of libavdevice, just ask, and if you'll read the answer I'll be happy to explain at lengths. If you have a diagnostic of an actual problem, please share it. If you have a detailed plan to make libavdevice's API significantly better, then please explain it, I am keenly interested. Another point: remember that in this project, we have a policy of no new API without user: we do not add a new function or API just for the fun of it; we add a new function or API when and if it allows to add a feature, to simplify existing code, etc. So, if you invent a new design for devices, and you implement an useful new feature thanks to this new design, or even better, several useful new features, then excellent. But if you propose to just add a grand new design for devices, and add a wrapper so that existing devices can be used unchanged, and another wrapper so that existing applications can still use the old API, and that's it, then you are making it worse. So, since AFAIK, you do not have plans to add new useful features to devices, or to libavfilter as a whole, my advice is: for now, leave it alone, until you have given it much more thought. (In return, I will continue to refrain from criticizing the design of the VAAPI stuff.) Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mxf: Establish register of local tags
Hi
Ticket #9079 brought this about. This should prevent accidentally
adding local tags that are not registered in the primer. It also allows
us to omit tags that we know won't be used, in a manner that is more
elegant than the old code.
The actual meat of this patch is mxf_mark_tag_unused(),
mxf_write_primer_pack(), mxf_write_local_tag() and
ff_mxf_lookup_local_tag()
fate-mxf passes of course, since this doesn't actually change the
output of the muxer.
/Tomas
From 1eb10ebb49103e30450b0aa6fed200ebdb5fe9ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?=
Date: Wed, 27 Jan 2021 14:08:55 +0100
Subject: [PATCH] avformat/mxf: Establish register of local tags
Tags can be marked "not used" upfront, saving some space in the primer.
av_asserts0() is used to enforce that only tags that are in the primer can actually be written.
---
libavformat/mxf.c| 144 +++-
libavformat/mxf.h| 3 +-
libavformat/mxfdec.c | 8 +-
libavformat/mxfenc.c | 517 +--
4 files changed, 349 insertions(+), 323 deletions(-)
diff --git a/libavformat/mxf.c b/libavformat/mxf.c
index 1901b24c68..4bc92a95fe 100644
--- a/libavformat/mxf.c
+++ b/libavformat/mxf.c
@@ -26,13 +26,140 @@ const uint8_t ff_mxf_mastering_display_prefix[13] = { 0x06,0x0e,0x2b,0
const uint8_t ff_mxf_random_index_pack_key[16] = { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x11,0x01,0x00 };
-/* be careful to update references to this array if reordering */
-/* local tags are dynamic and must not clash with others in mxfenc.c */
-const MXFLocalTagPair ff_mxf_mastering_display_local_tags[4] = {
+/**
+ * SMPTE RP210 http://www.smpte-ra.org/mdd/index.html
+ * https://smpte-ra.org/sites/default/files/Labels.xml
+ */
+const MXFLocalTagPair ff_mxf_local_tags[109] = {
+// preface set
+{ 0x3C0A, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x01,0x01,0x15,0x02,0x00,0x00,0x00,0x00}}, /* Instance UID */
+{ 0x3B02, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x02,0x04,0x00,0x00}}, /* Last Modified Date */
+{ 0x3B05, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x03,0x01,0x02,0x01,0x05,0x00,0x00,0x00}}, /* Version */
+{ 0x3B07, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x03,0x01,0x02,0x01,0x04,0x00,0x00,0x00}}, /* Object Model Version */
+{ 0x3B06, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x06,0x04,0x00,0x00}}, /* Identifications reference */
+{ 0x3B03, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x02,0x01,0x00,0x00}}, /* Content Storage reference */
+{ 0x3B09, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x03,0x00,0x00,0x00,0x00}}, /* Operational Pattern UL */
+{ 0x3B0A, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x10,0x02,0x01,0x00,0x00}}, /* Essence Containers UL batch */
+{ 0x3B0B, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x05,0x01,0x02,0x02,0x10,0x02,0x02,0x00,0x00}}, /* DM Schemes UL batch */
+// Identification
+{ 0x3C09, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x01,0x00,0x00,0x00}}, /* This Generation UID */
+{ 0x3C01, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x02,0x01,0x00,0x00}}, /* Company Name */
+{ 0x3C02, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x03,0x01,0x00,0x00}}, /* Product Name */
+{ 0x3C03, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x04,0x00,0x00,0x00}}, /* Product Version */
+{ 0x3C04, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x05,0x01,0x00,0x00}}, /* Version String */
+{ 0x3C05, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x07,0x00,0x00,0x00}}, /* Product ID */
+{ 0x3C06, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x02,0x03,0x00,0x00}}, /* Modification Date */
+{ 0x3C07, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x0A,0x00,0x00,0x00}}, /* Toolkit Version */
+{ 0x3C08, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x05,0x20,0x07,0x01,0x06,0x01,0x00,0x00}}, /* Platform */
+// Content Storage
+{ 0x1901, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x05,0x01,0x00,0x00}}, /* Package strong reference batch */
+{ 0x1902, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x04,0x05,0x02,0x00,0x00}}, /* Package strong reference batch */
+// Essence Container Data
+{ 0x2701, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x06,0x01,0x01,0x06,0x01,0x00,0x00,0x00}}, /* Linked Package UID */
+{ 0x3F07, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x04,0x01,0x03,0x04,0x04,0x00,0x00,0x00,0x00}}, /* BodySID */
+// Package
+{ 0x4401, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x01,0x01,0x01,0x15,0x10,0x00,0x00,0x00,0x00}}, /* Package UID */
+{ 0x4405, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x01,0x03,0x00,0x00}}, /* Package Creation Date */
+{ 0x4404, {0x06,0x0E,0x2B,0x34,0x01,0x01,0x01,0x02,0x07,0x02,0x01,0x10,0x0
Re: [FFmpeg-devel] [PATCH] avformat/utils: always preserve container dimensions for all streams
Quoting James Almer (2021-01-26 00:17:51) > On 1/25/2021 7:46 PM, Michael Niedermayer wrote: > > On Sun, Jan 24, 2021 at 11:41:13AM -0300, James Almer wrote: > >> If a decoder is used for probing it may change the dimensions reported by > >> the > >> demuxer, either by the lowres factor or because of assorted frames > >> reporting > >> different dimensions, and in a codec copy scenario, the last dimensions > >> arbitrarily set by it could end up being propagated to the muxer. > >> > >> Signed-off-by: James Almer > >> --- > >> libavformat/utils.c | 10 ++ > >> tests/ref/fate/cbs-vp9-vp90-2-05-resize | 2 +- > >> tests/ref/fate/redcode-demux| 2 +- > >> tests/ref/fate/wtv-demux| 4 ++-- > >> 4 files changed, 10 insertions(+), 8 deletions(-) > >> > > > > breaks: > > > > ./ffmpeg -i tickets/2892/MPEG_tbn_test.mov -c:v copy -c:a copy -vtag mx3n > > -timecode 10:00:00:00 -vframes 3 file.mov > > ... > > [mov @ 0x558785108e00] D-10/IMX must use 720x608 or 720x512 video resolution > > So the source mov is faulty and reports a wrong resolution? And trying > to pass it through instead of letting a decoder change it makes it fail > to mux because the muxer refuses to create non compliant files. > That means if there's no decoder in the build, you'd get the same result > as without this patch. > > The mere existence of this code here means that letting decoders set the > resolution in a codec copy scenario is not ideal, as shown by the fact > one can tell it to make up an arbitrary resolution like it's the case of > lowres, or it can just pick up one from an arbitrary frame. Hence > prioritizing what the demuxer reads from the container feels like the > correct thing to do. But of course, broken files are a thing. > > I don't know, maybe a new AVFMT_FLAG_ flag to choose between giving > priority to what the container reports or what a decoder sets, if any is > present, could work around this? I'd prefer an ffmpeg.c flag if anything. It is not lavf's job to set policy on what information is more trustworthy. The demuxer should export what is written in the container, the decoder/parser should export what is written in the codec. The user then decides which gets used. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] cbs_h2645: Implement replace-PS with a table rather than many functions
On Wed, Jan 27, 2021 at 7:06 AM Mark Thompson wrote:
>
> +
> +err = ff_cbs_make_unit_refcounted(ctx, unit);
> +if (err < 0)
> +return err;
> +
> +ref_array =
> + (AVBufferRef**)((uint8_t*)ctx->priv_data +
> ps_type->ref_array_offset);
> +ptr_array = (void**)((uint8_t*)ctx->priv_data +
> ps_type->ptr_array_offset);
> +active= (void**)((uint8_t*)ctx->priv_data +
> ps_type->active_offset);
> +
> +if (ptr_array[id] == *active) {
> +// The old active parameter set is being overwritten, so it can't
> +// be active after this point.
> +*active = NULL;
> +}
> +av_buffer_unref(&ref_array[id]);
> +
> +ref_array[id] = av_buffer_ref(unit->content_ref);
> +if (!ref_array[id])
> +return AVERROR(ENOMEM);
>
This happend after ff_cbs_make_unit_refcounted, do we need urnef
unit->content_ref
before return?
> +ptr_array[id] = ref_array[id]->data;
> +
> +return 0;
> +}
>
>
> 2.29.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".
___
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 3/9] avcodec: add SEI enum for vvc
On Tue, Jan 26, 2021 at 11:11 PM James Almer wrote:
> On 1/26/2021 12:09 PM, Nuo Mi wrote:
> > On Tue, Jan 26, 2021 at 10:36 PM James Almer wrote:
> >
> >> On 1/25/2021 11:15 AM, Nuo Mi wrote:
> >>> ---
> >>>libavcodec/vvc_sei.h | 47
>
> >>>1 file changed, 47 insertions(+)
> >>>create mode 100644 libavcodec/vvc_sei.h
> >>>
> >>> diff --git a/libavcodec/vvc_sei.h b/libavcodec/vvc_sei.h
> >>> new file mode 100644
> >>> index 00..90724669de
> >>> --- /dev/null
> >>> +++ b/libavcodec/vvc_sei.h
> >>> @@ -0,0 +1,47 @@
> >>> +/*
> >>> + * H.266/VVC Supplementary Enhancement Information messages
> >>> + *
> >>> + * 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
> >>> + */
> >>> +
> >>> +#ifndef AVCODEC_VVC_SEI_H
> >>> +#define AVCODEC_VVC_SEI_H
> >>> +
> >>> +/**
> >>> + * SEI message types
> >>> + */
> >>> +typedef enum {
> >>> +VVC_SEI_TYPE_BUFFERING_PERIOD = 0,
> >>> +VVC_SEI_TYPE_PICTURE_TIMING = 1,
> >>> +VVC_SEI_TYPE_PAN_SCAN_RECT= 2,
> >>> +VVC_SEI_TYPE_FILLER_PAYLOAD = 3,
> >>> +VVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35 = 4,
> >>> +VVC_SEI_TYPE_USER_DATA_UNREGISTERED = 5,
> >>> +VVC_SEI_TYPE_FILM_GRAIN_CHARACTERISTICS = 19,
> >>> +VVC_SEI_TYPE_FRAME_PACKING= 45,
> >>> +VVC_SEI_TYPE_PARAMETER_SETS_INCLUSION_INDICATION = 129,
> >>> +VVC_SEI_TYPE_DECODING_UNIT_INFO = 130,
> >>> +VVC_SEI_TYPE_DECODED_PICTURE_HASH = 132,
> >>> +VVC_SEI_TYPE_SCALABLE_NESTING = 133,
> >>> +VVC_SEI_TYPE_REGION_REFRESH_INFO = 134,
> >>> +VVC_SEI_TYPE_TIME_CODE= 136,
> >>> +VVC_SEI_TYPE_MASTERING_DISPLAY_INFO = 137,
> >>> +VVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO = 144,
> >>> +VVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
> >>> +} VVC_SEI_Type;
> >>> +
> >>> +#endif /* AVCODEC_VVC_SEI_H */
> >>
> >> This enum is no longer needed, since all the sei type enums have all
> >> been merged into sei.h
> >>
> > Yeah, but for vvc, 129 is not SEI_TYPE_ACTIVE_PARAMETER_SETS.
> > It's SEI_TYPE_PARAMETER_SETS_INCLUSION_INDICATION.
> > Maybe we can define it later.
>
> Yes, I sent a patch to add SEI_TYPE_PARAMETER_SETS_INCLUSION_INDICATION
> as an alias for SEI_TYPE_ACTIVE_PARAMETER_SETS last friday.
>
👍
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/6] avcodec/qsv_h2645: fix memory leak for plugin load
Quoting Xu Guangxin (2021-01-05 03:43:37)
> ---
> libavcodec/qsvdec_h2645.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
> index 02c41883b6..3d6e85230f 100644
> --- a/libavcodec/qsvdec_h2645.c
> +++ b/libavcodec/qsvdec_h2645.c
> @@ -69,6 +69,8 @@ static av_cold int qsv_decode_close(AVCodecContext *avctx)
> {
> QSVH2645Context *s = avctx->priv_data;
>
> +av_freep(&s->qsv.load_plugins);
Does this not get freed by av_opt_free()?
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avcodec/flashsv2enc: Fix use of uninitialized value
Before 257a83b969157eb76c18158a4e503e908d8b1125, certain buffers were
zero-allocated in the init function and only reallocated lateron if they
turned out to be too small; now they are only allocated during init,
leading to use-of-uninitialized values lateron. The same could happen
before if the dimensions are big enough so that the buffers would be
reallocated, as the new part of the reallocated buffer would not be
zeroed (happened for 960x960). So always zero the buffers in the
function designed to init them.
Signed-off-by: Andreas Rheinhardt
---
If no one objects, I'll send a patch to remove #ifndef FLASHSV2_DUMB stuff
lateron: It doesn't even compile any more and given that it has never
worked it stands to reason that any successfull non-dumb way needs to be
different from the currently outcommented code. Of course, I don't think
that anyone will ever add a successfull non-dumb way for this encoder
for an old format.
Furthermore, there are more bugs lurking in this code, namely the
ptr = av_realloc_array(ptr, size) which leads to memleaks on
reallocation failures as well as problems if the caller tries to call
the encoder lateron because block_width/height have already been set, so
that no reallocation attempt would be performed.
libavcodec/flashsv2enc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavcodec/flashsv2enc.c b/libavcodec/flashsv2enc.c
index 5139b17a28..430b6806c8 100644
--- a/libavcodec/flashsv2enc.c
+++ b/libavcodec/flashsv2enc.c
@@ -142,6 +142,7 @@ static void init_blocks(FlashSV2Context * s, Block * blocks,
{
int row, col;
Block *b;
+memset(blocks, 0, s->cols * s->rows * sizeof(*blocks));
for (col = 0; col < s->cols; col++) {
for (row = 0; row < s->rows; row++) {
b = blocks + (col + row * s->cols);
--
2.25.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add kirsch video filter
Will apply soon. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/3] avformat/dashdec: fix code style in is_common_init_section_exist
On Fri, Jan 15, 2021 at 20:13:44 +0800, Steven Liu wrote: > > Good idea, but the indentation is incorrect. The subsequent lines are > > not arguments to av_strcasecmp(), but further clauses for if(). > > > > Also, you should make this change the first commit of your series which > Hi Moritz, > > Do you mean this modify should be merged into the first patch? The style changes should be merged into one patch. (The first patch included a style change.) Moritz ___ 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/7] tools/target_dec_fuzzer: use non-obsolete decoding API
Quoting Michael Niedermayer (2021-01-02 02:24:42) > On Fri, Jan 01, 2021 at 02:37:21PM +0100, Anton Khirnov wrote: > > pushed patches 2-6, which nobody objected to > > > > Michael, could you please test 1/7? > > It seems still working with the patch with the 4 or so files i tried Thank you, patch pushed. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 6/6] avformat/flvdec: Check for EOF in loop in flv_data_packet()
Quoting Michael Niedermayer (2021-01-26 17:42:27)
> On Sun, Jan 24, 2021 at 02:17:05PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2021-01-23 23:10:56)
> > > Fixes: Timeout
> > > Fixes:
> > > 29656/clusterfuzz-testcase-minimized-ffmpeg_dem_FLV_fuzzer-5840098987999232
> > >
> > > Found-by: continuous fuzzing process
> > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > Signed-off-by: Michael Niedermayer
> > > ---
> > > libavformat/flvdec.c | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > > index 4bc5e15dd2..02797e1fba 100644
> > > --- a/libavformat/flvdec.c
> > > +++ b/libavformat/flvdec.c
> > > @@ -909,6 +909,11 @@ static int flv_data_packet(AVFormatContext *s,
> > > AVPacket *pkt,
> > >
> > > while (array || (ret = amf_get_string(pb, buf, sizeof(buf))) > 0) {
> > > AMFDataType type = avio_r8(pb);
> > > +if (avio_feof(pb)) {
> > > +ret = AVERROR_INVALIDDATA;
> > > +goto skip;
> > > +}
> > > +
> > > if (type == AMF_DATA_TYPE_STRING && (array || !strcmp(buf,
> > > "text"))) {
> > > length = avio_rb16(pb);
> > > ret= av_get_packet(pb, pkt, length);
> > > --
> > > 2.17.1
> >
> > IMO it would be cleaner to make amf_get_string() check the return value
> > of avio_read() and return an error on short reads.
>
> will do but that does not fix the issue as array is set and
> amf_get_string() does not run in that case.
That makes me wonder whether it would not be better to make
av_get_packet() return an error on short reads. Is there any use case
in lavf where we would want to return a partial packet?
--
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avpacket: RFC for ABI bump additions
Quoting James Almer (2021-01-26 20:11:16) > On 1/26/2021 1:17 PM, Anton Khirnov wrote: > > We could start by adding a field to AVPacket that would be set to a > > magic value by av_packet_alloc(). > > Then have e.g. AVCodecContext/AVFormatContext warn when they see a > > packet without this magic value. > > I don't like much the idea of adding a public field just to emit a > deprecation warning. int internal_do_not_touch; // do not touch is not really public. It is visible in the public headers, but so are all the AVFooInternal. I agree that it is not the prettiest thing ever, but it's not too bad. And I believe it would solve a real problem, since we have few other ways to let our users know they need to change something. Most of them do not follow development closely, I'd think. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 20/39] avcodec/h261dec: Don't initialize unused part of RLTable
Quoting Andreas Rheinhardt (2021-01-23 19:50:11) > Anton Khirnov: > > Quoting Andreas Rheinhardt (2021-01-21 21:20:52) > >> Anton Khirnov: > >>> Quoting Andreas Rheinhardt (2020-12-10 12:16:38) > The H.261 decoder only uses an RLTable's VLC table, yet it also > initializes its index_run, max_level and max_run. This commit stops > doing so; it will also simplify making this decoder init-threadsafe, > as the H.261 decoder and encoder now initialize disjoint parts of their > common RLTable. > >>> > >>> Does it then make sense to keep this RLTable common? > >>> > >> I presume you want to know whether the RLTable structure should be split > >> into smaller structures? > > > > No, what I meant was whether we shouldn't use different RLTable > > instances for encoder and decoder, since their use is disjoint. That > > would make the code easier to reason about. > > I actually didn't think that these RLTables were difficult to reason > about: ff_rl_init and ff_rl_init_vlc/ff_init_2d_vlc_rl initialize > different parts of an RLTable and both only use the static parts of an > RLTable, so that these two can be called independently. In particular > there is no clash in the case of H.261 after the unnecessary call to > ff_rl_init by the decoder is gone. And in case the decoder needs > ff_rl_init, too, one just needs to make sure that it is only initialized > once and that is really not onerous. > > So, my answer to your original question is that it makes sense to keep > these RLTables common. But then you have to remember that this is true. Someone who does not would expect there to be one lock per object. So if you don't want to unshare the table, a comment in the code would be useful. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter: add colortemperature filter
On Tue, Jan 26, 2021 at 12:16:31PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol
> ---
> doc/filters.texi | 22 +++
> libavfilter/Makefile | 1 +
> libavfilter/allfilters.c | 1 +
> libavfilter/vf_colortemperature.c | 280 ++
> 4 files changed, 304 insertions(+)
> create mode 100644 libavfilter/vf_colortemperature.c
This breaks build on arm
In file included from src/libavutil/intmath.h:30:0,
from src/libavutil/common.h:114,
from src/libavutil/avutil.h:296,
from src/libavutil/opt.h:31,
from src/libavfilter/vf_colortemperature.c:23:
src/libavutil/arm/intmath.h: In function ‘temperature_slice16’:
src/libavutil/arm/intmath.h:77:5: warning: asm operand 2 probably doesn’t match
constraints
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/libavutil/arm/intmath.h:77:5: warning: asm operand 2 probably doesn’t match
constraints
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/libavutil/arm/intmath.h:77:5: warning: asm operand 2 probably doesn’t match
constraints
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/libavutil/arm/intmath.h:77:5: error: impossible constraint in ‘asm’
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/libavutil/arm/intmath.h:77:5: error: impossible constraint in ‘asm’
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/libavutil/arm/intmath.h:77:5: error: impossible constraint in ‘asm’
__asm__ ("usat %0, %2, %1" : "=r"(x) : "r"(a), "i"(p));
^~~
src/ffbuild/common.mak:67: recipe for target
'libavfilter/vf_colortemperature.o' failed
make: *** [libavfilter/vf_colortemperature.o] Error 1
make: *** Waiting for unfinished jobs
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
