Re: [FFmpeg-devel] avformat/mxfenc: SMPTE RDD 48:2018 Amd 1:2022 (FFV1 in MXF) support

[Dave Rice]

> On Jan 20, 2023, at 10:17 AM, Tomas Härdin  wrote:
> 
> ons 2023-01-18 klockan 15:15 +0100 skrev Jerome Martinez:
>> On 18/01/2023 14:40, Tomas Härdin wrote:
>>> Creating a new subthread because I just noticed something
>> 
>> I am a bit lost there because the line of code below is not part of
>> this 
>> FFV1 patch.
>> Additionally, none on my patches (FFV1 of MXF
>> stored/sampled/displayed 
>> fix) modifies the discussed behavior (FFmpeg behavior would be same 
>> before and after this patch for MPEG-2 and AVC), so should not block
>> any 
>> of them, and a potential fix for that should have its own patch as it
>> would be a separate issue.
> 
> True, it doesn't need to hold up this patch. But some discussion is
> warranted I think. I might create a separate patchset for this.
> 
>> 
>> Anyway:
>> 
>> 
>>> 
>>>> +//Stored height
>>>>   mxf_write_local_tag(s, 4, 0x3202);
>>>>   avio_wb32(pb, stored_height>>sc->interlaced);
>>>> 
>>> Won't this be incorrect for files whose dimensions are multiples of
>>> 16
>>> but not multiples of 32? Isn't each field stored separately with
>>> dimensions a multiple of 16? So while for 1080p we'll have
>>> 
>>>StoredHeight = 1088
>>>SampledHeight = 1080
>>> 
>>> and 1080i:
>>> 
>>>StoredHeight = 544
>>>SampledHeight = 540
>>> 
>>> Where 544 is a multiple of 16, for say 720p we have
>>> 
>>>StoredHeight = 720
>>>SampledHeight = 720
>>> 
>>> but for a hypothetical 720i we'd get
>>> 
>>>StoredHeight = 360
>>>SampledHeight = 360
>>> 
>>> whereas the correct values should be
>>> 
>>>StoredHeight = 368
>>>SampledHeight = 360
>> 
>> AFAIK, it would depend about if the stream has a picture_structure
>> frame 
>> (16x16 applies to the frame?) of field (16x16 applies to the field?),
>> but I really don't know enough there for having a relevant opinion.
>> 
>> I can just say that I don't change the behavior of FFmpeg in your use
>> case, I found the issues when I tried a random width and height of
>> FFV1 
>> stream then checked with MPEG-2 Video and the sampled width was wrong
>> for sure e.g. sampled width of 1920 for a stream having a width of
>> 1912, 
>> with current FFmpeg code, and for your use case I am sure about
>> nothing 
>> so I don't change the behavior with my patch, IMO if there is an
>> issue 
>> with 720i MPEG-2 Video it should be in a separate topic and patch as
>> it 
>> would modify the "stored_height = (st->codecpar->height+15)/16*16" 
>> current code (in my patch I just move this code), unless we are sure
>> of 
>> what should be changed on this side and apply a fix on the way.
>> Better 
>> to fix 1 issue and let 1 open with no change than fixing no issue 
>> because we wouldn't be sure for 1 of the 2.
> 
> I suspect we are lucky because 720i doesn't really exist in the real
> world, and 576i and 480i are both multiples of 32.
> 
> IMO mxfenc shouldn't lie, but looking at S377m StoredWidth/Height are
> "best effort" and thus shall be encoded. Their values will depend on
> FrameLayout which in turn depends on what you say - how exactly the
> interlacing is done.
> 
> TL;DR: this patchset doesn't need to be held up by this.

I'm just nudging on the consideration of merging this patch. I've been testing 
it over the last week with ffv1/mxf content and have found this demuxing 
support very helpful.
Dave Rice
___
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/3] avformat/mxfdec: SMPTE RDD 48:2018 support

[Dave Rice]

Y(uid, mxf_ffv1_extradata)) {
> +if (ffv1_sub_descriptor->extradata)
> +av_log(NULL, AV_LOG_WARNING, "Duplicate ffv1_extradata\n");
> +av_free(ffv1_sub_descriptor->extradata);
> +ffv1_sub_descriptor->extradata_size = 0;
> +ffv1_sub_descriptor->extradata = av_malloc(size);
> +if (!ffv1_sub_descriptor->extradata)
> +return AVERROR(ENOMEM);
> +ffv1_sub_descriptor->extradata_size = size;
> +avio_read(pb, ffv1_sub_descriptor->extradata, size);
> +}
> +
> +return 0;
> +}
> +
> static int mxf_read_indirect_value(void *arg, AVIOContext *pb, int size)
> {
> MXFTaggedValue *tagged_value = arg;
> @@ -1554,6 +1583,7 @@ static const MXFCodecUL 
> mxf_picture_essence_container_uls[] = {
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x1c,0x01,0x00
>  }, 14, AV_CODEC_ID_PRORES, NULL, 14 }, /* ProRes */
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x02,0x0d,0x01,0x03,0x01,0x02,0x04,0x60,0x01
>  }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15 }, /* MPEG-ES */
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x01,0x04,0x01
>  }, 14, AV_CODEC_ID_MPEG2VIDEO, NULL, 15, D10D11Wrap }, /* SMPTE D-10 mapping 
> */
> +{ { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0d,0x0d,0x01,0x03,0x01,0x02,0x23,0x01,0x00
>  }, 14,   AV_CODEC_ID_FFV1, NULL, 14 },
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x02,0x41,0x01
>  }, 14,AV_CODEC_ID_DVVIDEO, NULL, 15 }, /* DV 625 25mbps */
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x01,0x0d,0x01,0x03,0x01,0x02,0x05,0x00,0x00
>  }, 14,   AV_CODEC_ID_RAWVIDEO, NULL, 15, RawVWrap }, /* uncompressed picture 
> */
> { { 
> 0x06,0x0e,0x2b,0x34,0x04,0x01,0x01,0x0a,0x0e,0x0f,0x03,0x01,0x02,0x20,0x01,0x01
>  }, 15, AV_CODEC_ID_HQ_HQA },
> @@ -2444,6 +2474,21 @@ static MXFMCASubDescriptor 
> *find_mca_link_id(MXFContext *mxf, enum MXFMetadataSe
> return NULL;
> }
> 
> +static void parse_ffv1_sub_descriptor(MXFContext *mxf, MXFTrack 
> *source_track, MXFDescriptor *descriptor, AVStream *st)
> +{
> +for (int i = 0; i < descriptor->sub_descriptors_count; i++) {
> +MXFFFV1SubDescriptor *ffv1_sub_descriptor = 
> mxf_resolve_strong_ref(mxf, >sub_descriptors_refs[i], 
> FFV1SubDescriptor);
> +if (ffv1_sub_descriptor == NULL)
> +continue;
> +
> +descriptor->extradata  = ffv1_sub_descriptor->extradata;
> +descriptor->extradata_size = ffv1_sub_descriptor->extradata_size;
> +ffv1_sub_descriptor->extradata = NULL;
> +ffv1_sub_descriptor->extradata_size = 0;
> +break;
> +}
> +}
> +
> static int parse_mca_labels(MXFContext *mxf, MXFTrack *source_track, 
> MXFDescriptor *descriptor, AVStream *st)
> {
> uint64_t routing[FF_SANE_NB_CHANNELS] = {0};
> @@ -2972,6 +3017,8 @@ static int mxf_parse_structural_metadata(MXFContext 
> *mxf)
> st->codecpar->codec_id = AV_CODEC_ID_EIA_608;
> }
> }
> +if (!descriptor->extradata)
> +parse_ffv1_sub_descriptor(mxf, source_track, descriptor, st);
> if (descriptor->extradata) {
> if (!ff_alloc_extradata(st->codecpar, 
> descriptor->extradata_size)) {
> memcpy(st->codecpar->extradata, descriptor->extradata, 
> descriptor->extradata_size);
> @@ -3159,6 +3206,7 @@ static const MXFMetadataReadTableEntry 
> mxf_metadata_read_table[] = {
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6b,0x00
>  }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), 
> AudioChannelLabelSubDescriptor },
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6c,0x00
>  }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), 
> SoundfieldGroupLabelSubDescriptor },
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x6d,0x00
>  }, mxf_read_mca_sub_descriptor, sizeof(MXFMCASubDescriptor), 
> GroupOfSoundfieldGroupsLabelSubDescriptor },
> +{ { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x81,0x03
>  }, mxf_read_ffv1_sub_descriptor, sizeof(MXFFFV1SubDescriptor), 
> FFV1SubDescriptor },
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3A,0x00
>  }, mxf_read_track, sizeof(MXFTrack), Track }, /* Static Track */
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x3B,0x00
>  }, mxf_read_track, sizeof(MXFTrack), Track }, /* Generic Track */
> { { 
> 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x14,0x00
>  }, mxf_read_timecode_component, sizeof(MXFTimecodeComponent), 
> TimecodeComponent },
> -- 
> 2.17.1

For those interested, this patch supports SMPTE RDD48 which is at 
https://www.digitizationguidelines.gov/guidelines/rdd48-2018_published.pdf 
<https://www.digitizationguidelines.gov/guidelines/rdd48-2018_published.pdf> 
and is one of SMPTE’s first documents published with a Creative Commons 
license. 
Dave Rice

___
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] avcodec: Add dv marker bsf

[Dave Rice]

> On Mar 12, 2022, at 1:09 PM, Michael Niedermayer  
> wrote:
> 
> On Sat, Mar 12, 2022 at 10:11:52AM -0500, Dave Rice wrote:
>> 
>> 
>>> On Mar 10, 2022, at 4:41 AM, Tobias Rapp  wrote:
>>> 
>>> On 09/03/2022 19:18, Michael Niedermayer wrote:
>>>> Signed-off-by: Michael Niedermayer 
>>>> ---
>>>> doc/bitstream_filters.texi   |  30 
>>>> libavcodec/Makefile  |   1 +
>>>> libavcodec/bitstream_filters.c   |   1 +
>>>> libavcodec/dv_error_marker_bsf.c | 127 +++
>>>> 4 files changed, 159 insertions(+)
>>>> create mode 100644 libavcodec/dv_error_marker_bsf.c
>>>> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
>>>> index a0092878c8..8c5d84dceb 100644
>>>> --- a/doc/bitstream_filters.texi
>>>> +++ b/doc/bitstream_filters.texi
>>>> @@ -132,6 +132,36 @@ the header stored in extradata to the key packets:
>>>> ffmpeg -i INPUT -map 0 -flags:v +global_header -c:v libx264 -bsf:v 
>>>> dump_extra out.ts
>>>> @end example
>>>> +@section dv_error_marker
>>>> +
>>>> +Blocks in DV which are marked as damaged are replaced by blocks of the 
>>>> specified color.
>>>> +
>>>> +@table @option
>>>> +@item color
>>>> +The color to replace damaged blocks by
>>>> +@item sta
>>>> +A 16 bit mask which specifies which of the 16 possible error status 
>>>> values are
>>>> +to be replaced by colored blocks. 0xFFFE is the default which replaces 
>>>> all non 0
>>>> +error status values.
>>>> +@table @samp
>>>> +@item ok
>>>> +No error, no concealment
>>>> +@item err
>>>> +Error, No concealment
>>>> +@item res
>>>> +Reserved
>>>> +@item notok
>>>> +Error or concealment
>>>> +@item notres
>>>> +Not reserved
>>>> +@item Aa, Ba, Ca, Ab, Bb, Cb, A, B, C, a, b, erri, erru
>>>> +The specific error status code
>>>> +@end table
>>>> +see page 44-46 or section 5.5 of
>>>> +@url{http://web.archive.org/web/20060927044735/http://www.smpte.org/smpte_store/standards/pdf/s314m.pdf}
>>>> +
>>>> +@end table
>>>> +
>>>> @section eac3_core
>>>> [...]
>>> The filter options look nice to me now. Have not actually tested the 
>>> bitstream filter on DV files, though.
>> 
>> I tested this and this works well for me. Here's a few samples that 
>> demonstrate the filter:
>> 
>> ./ffmpeg -i 
>> https://samples.ffmpeg.org/archive/audio/pcm_s16le/dv+dvvideo+pcm_s16le++dropout.dv
>>   -bsf dv_error_marker=sta=b -f rawvideo -c:v copy - | ffplay -
>> ./ffmpeg -i 
>> https://archive.org/download/DvAnalyzerSampleDvVideoErrorConcealment/DV_Analyzer_Sample_Video_Error_Concealment_original.dv
>>  -bsf dv_error_marker=sta=b -f rawvideo -c:v copy - | ffplay -
> 
> I tested a bit more and it failed with dvcprohd, i have fixed it and will in a
> moment post a version that seems to work with both
> please retest

I retested the new version on variety of DV25 and DV50 content. Looks good to 
me.

> PS: i used some artificially damaged files from fate/dv/
> 
> thx
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Good people do not need laws to tell them to act responsibly, while bad
> people will find a way around the laws. -- Plato
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel 
> <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org <mailto:ffmpeg-devel-requ...@ffmpeg.org> with 
> subject "unsubscribe".

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

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

Re: [FFmpeg-devel] [PATCH v2] avcodec: Add dv marker bsf

[Dave Rice]

> On Mar 10, 2022, at 4:41 AM, Tobias Rapp  wrote:
> 
> On 09/03/2022 19:18, Michael Niedermayer wrote:
>> Signed-off-by: Michael Niedermayer 
>> ---
>>  doc/bitstream_filters.texi   |  30 
>>  libavcodec/Makefile  |   1 +
>>  libavcodec/bitstream_filters.c   |   1 +
>>  libavcodec/dv_error_marker_bsf.c | 127 +++
>>  4 files changed, 159 insertions(+)
>>  create mode 100644 libavcodec/dv_error_marker_bsf.c
>> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
>> index a0092878c8..8c5d84dceb 100644
>> --- a/doc/bitstream_filters.texi
>> +++ b/doc/bitstream_filters.texi
>> @@ -132,6 +132,36 @@ the header stored in extradata to the key packets:
>>  ffmpeg -i INPUT -map 0 -flags:v +global_header -c:v libx264 -bsf:v 
>> dump_extra out.ts
>>  @end example
>>  +@section dv_error_marker
>> +
>> +Blocks in DV which are marked as damaged are replaced by blocks of the 
>> specified color.
>> +
>> +@table @option
>> +@item color
>> +The color to replace damaged blocks by
>> +@item sta
>> +A 16 bit mask which specifies which of the 16 possible error status values 
>> are
>> +to be replaced by colored blocks. 0xFFFE is the default which replaces all 
>> non 0
>> +error status values.
>> +@table @samp
>> +@item ok
>> +No error, no concealment
>> +@item err
>> +Error, No concealment
>> +@item res
>> +Reserved
>> +@item notok
>> +Error or concealment
>> +@item notres
>> +Not reserved
>> +@item Aa, Ba, Ca, Ab, Bb, Cb, A, B, C, a, b, erri, erru
>> +The specific error status code
>> +@end table
>> +see page 44-46 or section 5.5 of
>> +@url{http://web.archive.org/web/20060927044735/http://www.smpte.org/smpte_store/standards/pdf/s314m.pdf}
>> +
>> +@end table
>> +
>>  @section eac3_core
>> [...]
> The filter options look nice to me now. Have not actually tested the 
> bitstream filter on DV files, though.

I tested this and this works well for me. Here's a few samples that demonstrate 
the filter:

./ffmpeg -i 
https://samples.ffmpeg.org/archive/audio/pcm_s16le/dv+dvvideo+pcm_s16le++dropout.dv
  -bsf dv_error_marker=sta=b -f rawvideo -c:v copy - | ffplay -
./ffmpeg -i 
https://archive.org/download/DvAnalyzerSampleDvVideoErrorConcealment/DV_Analyzer_Sample_Video_Error_Concealment_original.dv
 -bsf dv_error_marker=sta=b -f rawvideo -c:v copy - | ffplay -

Dave Rice

___
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 conduct

[Dave Rice]

Hello FFmpeg community,

I'm writing on behalf of FFmpeg's code of conduct committee to acknowledge that 
the tone on the mailing list has recently become more discouraging, 
challenging, and argumentative, notably on subjects related to subtitles from 
multiple people. We should be able to do better.

We'd like to remind everyone of the Code of Conduct, 
http://ffmpeg.org/developer.html#Code-of-conduct 
<http://ffmpeg.org/developer.html#Code-of-conduct>, of this community, which 
reminds of our aspirational values of being considerate, cooperative, and not 
putting one another down. From reading the exchanges ongling on ffmpeg-devel 
and reading the code of conduct, we think there is a clear mismatch here.

This reminder may be needed now and then, but please, let's all make our 
community a better place than this. Let's please work to support and encourage 
one another, offer fair critical reviews, and work collaboratively towards 
rough consensus.

Kind Regards,

The CC
James Almer
Thilo Borgmann
Carl Eugen Hoyos
Jean-Baptiste Kempf
    Dave Rice
___
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/dv: always set audio packet duration

[Dave Rice]

> On Sep 6, 2021, at 2:28 PM, Andreas Rheinhardt 
>  wrote:
> 
> Paul B Mahol:
>> If audio packet is present in DV stream it have duration of 1 in DV timebase 
>> units.
>> 
>> Signed-off-by: Paul B Mahol 
>> ---
>> libavformat/dv.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/libavformat/dv.c b/libavformat/dv.c
>> index d7909683c3..b2b74162df 100644
>> --- a/libavformat/dv.c
>> +++ b/libavformat/dv.c
>> @@ -48,6 +48,7 @@ struct DVPacket {
>> int  stream_index;
>> int  flags;
>> int64_t  pos;
>> +int64_t  duration;
>> };
>> 
>> struct DVDemuxContext {
>> @@ -276,6 +277,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, 
>> const uint8_t *frame)
>> c->audio_pkt[i].stream_index = c->ast[i]->index;
>> c->audio_pkt[i].flags   |= AV_PKT_FLAG_KEY;
>> c->audio_pkt[i].pts  = AV_NOPTS_VALUE;
>> +c->audio_pkt[i].duration = 0;
>> c->audio_pkt[i].pos  = -1;
>> }
>> c->ast[i]->codecpar->sample_rate= dv_audio_frequency[freq];
>> @@ -374,6 +376,7 @@ int avpriv_dv_get_packet(DVDemuxContext *c, AVPacket 
>> *pkt)
>> pkt->stream_index = c->audio_pkt[i].stream_index;
>> pkt->flags= c->audio_pkt[i].flags;
>> pkt->pts  = c->audio_pkt[i].pts;
>> +pkt->duration = c->audio_pkt[i].duration;
>> pkt->pos  = c->audio_pkt[i].pos;
>> 
>> c->audio_pkt[i].size = 0;
>> @@ -404,6 +407,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, AVPacket 
>> *pkt,
>> c->audio_pkt[i].pos  = pos;
>> c->audio_pkt[i].size = size;
>> c->audio_pkt[i].pts  = (c->sys->height == 720) ? (c->frames & ~1) : 
>> c->frames;
>> +c->audio_pkt[i].duration = 1;
>> ppcm[i] = c->audio_buf[i];
>> }
>> if (c->ach)
>> 
> Sure about that? According to the code, the packets contain slightly
> different amounts of samples; see dv_extract_audio_info(), in particular
> lines 242 and 289.
> (It seems to me that one audio packet is supposed to contain the audio
> for one video frame, yet the standard sample rates and standard video
> framerates (in particular NTSC) are incompatible, so a slight variation
> in the number of samples per frame is used.)
> IMO one should set the audio timebase to be the inverse of the
> samplerate and set the duration based upon the number of samples. But I
> have zero experience with dv.

Yes the audio duration is variable in NTSC DV, see page 17 of 
http://web.archive.org/web/20060927044735/http://www.smpte.org/smpte_store/standards/pdf/s314m.pdf.
 Each frame has either 1600 or 1602 samples, so while the video is 1001/3 
the audio is either 1600/48000 or 1602/48000. So video at ~0.0333667 and 
audio at either ~0.033 or 0.033375.

I’ve tested this patch and it does correct some errors. For instance, before 
this patch frame 3 of the output only offers half of the needed duration, so 
the resulting audio track is (1001/3)/2 shorter than the video. But after 
the patch, the video and audio outputs have the same length.

ffmpeg -i https://archive.org/download/test_a_202108/TESTTHIS.dv 
-filter_complex "[0:a:0]aresample=async=1:min_hard_comp=0.01[aud]" -c:v:0 copy 
-map 0:v:0 -c:a pcm_s16le -map "[aud]” output.mov

Dave Rice

___
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/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

Hi Marton,

> On Feb 23, 2021, at 3:07 PM, Dave Rice  wrote:
> 
>> On Feb 23, 2021, at 2:42 PM, Marton Balint  wrote:
>> 
>> On Sat, 20 Feb 2021, Dave Rice wrote:
>> 
>>> Hi,
>>> 
>>>> On Oct 31, 2020, at 5:15 PM, Marton Balint >>> <mailto:c...@passwd.hu>> wrote:
>>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>>>> On Oct 31, 2020, at 3:47 PM, Marton Balint >>>>> <mailto:c...@passwd.hu>> wrote:
>>>>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>>>>> Hi Marton,
>>>>>>>> On Oct 31, 2020, at 12:56 PM, Marton Balint >>>>>>> <mailto:c...@passwd.hu>> wrote:
>>>>>>>> Fixes out of sync timestamps in ticket #8762.
>>>>>>> Although Michael’s recent patch does address the issue documented in 
>>>>>>> 8762, I haven’t found this patch to fix the issue. I tried with -c:a 
>>>>>>> copy and with -c:a pcm_s16le with some sample files that exhibit this 
>>>>>>> issue but each output was out of sync. I put an output at 
>>>>>>> https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795 
>>>>>>> <https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795>. 
>>>>>>> That output notes that 3597 packages of video are read and 3586 packets 
>>>>>>> of audio. In the resulting file, at the end of the timeline the audio 
>>>>>>> is 9 frames out of sync and my output video stream is 00:02:00.020 and 
>>>>>>> output audio stream is 00:01:59.653.
>>>>>>> Beyond copying or encoding the audio, are there other options I should 
>>>>>>> use to test this?
>>>>>> Well, it depends on what you want. After this patch you should get a 
>>>>>> file which has audio packets synced to video, but the audio stream is 
>>>>>> sparse, not every video packet has a corresponding audio packet. (It 
>>>>>> looks like our MOV muxer does not support muxing of sparse audio 
>>>>>> therefore does not produce proper timestamps. But MKV does, please try 
>>>>>> that.)
>>>>>> You can also make ffmpeg generate the missing audio based on packet 
>>>>>> timestamps. Swresample has an async=1 option, so something like this 
>>>>>> should get you synced audio with continous audio packets:
>>>>>> ffmpeg -y -i 167052_12.dv -c:v copy \
>>>>>> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov
>>>>> Thank you for this. With the patch and async, the result is synced and 
>>>>> the resulting audio was the same as Michael’s patch.
>>>>> Could you explain why you used min_hard_comp here? IIUC min_hard_comp is 
>>>>> a set a threshold between the strategies of trim/fill or stretch/squeeze 
>>>>> to align the audio to time; however, the async documentation says 
>>>>> "Setting this to 1 will enable filling and trimming, larger values 
>>>>> represent the maximum amount in samples that the data may be stretched or 
>>>>> squeezed” so I thought that async=1 would not permit stretch/squeeze 
>>>>> anyway.
>>>> It is documented poorly, but if you check the source code you will see 
>>>> that async=1 implicitly sets min_comp to 0.001 enabling trimming/dropping. 
>>>> min_hard_comp decides the threshold when silence injection actually 
>>>> happens, and the default for that is 0.1, which is more than a frame, 
>>>> therefore not acceptable if we want to maintain <1 frame accuracy. Or at 
>>>> least that is how I think it should work.
>>> 
>> 
>>> I’ve found that aresample=async=1:min_hard_comp=0.01, as discussed here, 
>>> works well to add audio samples to maintain timestamp accuracy when muxing 
>>> into a format like mov. However, this approach doesn’t work if the 
>>> sparseness of the audio stream is at the end of the stream. Is there a way 
>>> to use min_hard_comp to consider differences between a timestamp and audio 
>>> data when one of the ends of that range is the end of the file?
>> 
>> I am not aware of a smart method to generate missing audio in the end until 
>> the end of video.
>> 
>> As a possible workaround you may query the video length using
>> ffprobe or mediainfo, and then use a second filter, apad to pad audio:
>> 
&g

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Feb 23, 2021, at 2:42 PM, Marton Balint  wrote:
> 
> 
> 
> On Sat, 20 Feb 2021, Dave Rice wrote:
> 
>> Hi,
>> 
>>> On Oct 31, 2020, at 5:15 PM, Marton Balint >> <mailto:c...@passwd.hu>> wrote:
>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>>> On Oct 31, 2020, at 3:47 PM, Marton Balint >>>> <mailto:c...@passwd.hu>> wrote:
>>>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>>>> Hi Marton,
>>>>>>> On Oct 31, 2020, at 12:56 PM, Marton Balint >>>>>> <mailto:c...@passwd.hu>> wrote:
>>>>>>> Fixes out of sync timestamps in ticket #8762.
>>>>>> Although Michael’s recent patch does address the issue documented in 
>>>>>> 8762, I haven’t found this patch to fix the issue. I tried with -c:a 
>>>>>> copy and with -c:a pcm_s16le with some sample files that exhibit this 
>>>>>> issue but each output was out of sync. I put an output at 
>>>>>> https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795 
>>>>>> <https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795>. That 
>>>>>> output notes that 3597 packages of video are read and 3586 packets of 
>>>>>> audio. In the resulting file, at the end of the timeline the audio is 9 
>>>>>> frames out of sync and my output video stream is 00:02:00.020 and output 
>>>>>> audio stream is 00:01:59.653.
>>>>>> Beyond copying or encoding the audio, are there other options I should 
>>>>>> use to test this?
>>>>> Well, it depends on what you want. After this patch you should get a file 
>>>>> which has audio packets synced to video, but the audio stream is sparse, 
>>>>> not every video packet has a corresponding audio packet. (It looks like 
>>>>> our MOV muxer does not support muxing of sparse audio therefore does not 
>>>>> produce proper timestamps. But MKV does, please try that.)
>>>>> You can also make ffmpeg generate the missing audio based on packet 
>>>>> timestamps. Swresample has an async=1 option, so something like this 
>>>>> should get you synced audio with continous audio packets:
>>>>> ffmpeg -y -i 167052_12.dv -c:v copy \
>>>>> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov
>>>> Thank you for this. With the patch and async, the result is synced and the 
>>>> resulting audio was the same as Michael’s patch.
>>>> Could you explain why you used min_hard_comp here? IIUC min_hard_comp is a 
>>>> set a threshold between the strategies of trim/fill or stretch/squeeze to 
>>>> align the audio to time; however, the async documentation says "Setting 
>>>> this to 1 will enable filling and trimming, larger values represent the 
>>>> maximum amount in samples that the data may be stretched or squeezed” so I 
>>>> thought that async=1 would not permit stretch/squeeze anyway.
>>> It is documented poorly, but if you check the source code you will see that 
>>> async=1 implicitly sets min_comp to 0.001 enabling trimming/dropping. 
>>> min_hard_comp decides the threshold when silence injection actually 
>>> happens, and the default for that is 0.1, which is more than a frame, 
>>> therefore not acceptable if we want to maintain <1 frame accuracy. Or at 
>>> least that is how I think it should work.
>> 
> 
>> I’ve found that aresample=async=1:min_hard_comp=0.01, as discussed here, 
>> works well to add audio samples to maintain timestamp accuracy when muxing 
>> into a format like mov. However, this approach doesn’t work if the 
>> sparseness of the audio stream is at the end of the stream. Is there a way 
>> to use min_hard_comp to consider differences between a timestamp and audio 
>> data when one of the ends of that range is the end of the file?
> 
> I am not aware of a smart method to generate missing audio in the end until 
> the end of video.
> 
> As a possible workaround you may query the video length using
> ffprobe or mediainfo, and then use a second filter, apad to pad audio:
> 
> -af aresample=async=1:min_hard_comp=0.01,apad=whole_dur=
> 
> Tnis might do what you want, but requires an additional step to query the 
> video length…


[…]
Perfect, thanks for sharing this idea.
Dave

___
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/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

Hi,

> On Oct 31, 2020, at 5:15 PM, Marton Balint  <mailto:c...@passwd.hu>> wrote:
> On Sat, 31 Oct 2020, Dave Rice wrote:
>>> On Oct 31, 2020, at 3:47 PM, Marton Balint >> <mailto:c...@passwd.hu>> wrote:
>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>> Hi Marton,
>>>>> On Oct 31, 2020, at 12:56 PM, Marton Balint >>>> <mailto:c...@passwd.hu>> wrote:
>>>>> Fixes out of sync timestamps in ticket #8762.
>>>> Although Michael’s recent patch does address the issue documented in 8762, 
>>>> I haven’t found this patch to fix the issue. I tried with -c:a copy and 
>>>> with -c:a pcm_s16le with some sample files that exhibit this issue but 
>>>> each output was out of sync. I put an output at 
>>>> https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795 
>>>> <https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795>. That 
>>>> output notes that 3597 packages of video are read and 3586 packets of 
>>>> audio. In the resulting file, at the end of the timeline the audio is 9 
>>>> frames out of sync and my output video stream is 00:02:00.020 and output 
>>>> audio stream is 00:01:59.653.
>>>> Beyond copying or encoding the audio, are there other options I should use 
>>>> to test this?
>>> Well, it depends on what you want. After this patch you should get a file 
>>> which has audio packets synced to video, but the audio stream is sparse, 
>>> not every video packet has a corresponding audio packet. (It looks like our 
>>> MOV muxer does not support muxing of sparse audio therefore does not 
>>> produce proper timestamps. But MKV does, please try that.)
>>> You can also make ffmpeg generate the missing audio based on packet 
>>> timestamps. Swresample has an async=1 option, so something like this should 
>>> get you synced audio with continous audio packets:
>>> ffmpeg -y -i 167052_12.dv -c:v copy \
>>> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov
>> 
>> Thank you for this. With the patch and async, the result is synced and the 
>> resulting audio was the same as Michael’s patch.
>> 
>> Could you explain why you used min_hard_comp here? IIUC min_hard_comp is a 
>> set a threshold between the strategies of trim/fill or stretch/squeeze to 
>> align the audio to time; however, the async documentation says "Setting this 
>> to 1 will enable filling and trimming, larger values represent the maximum 
>> amount in samples that the data may be stretched or squeezed” so I thought 
>> that async=1 would not permit stretch/squeeze anyway.
> 
> It is documented poorly, but if you check the source code you will see that 
> async=1 implicitly sets min_comp to 0.001 enabling trimming/dropping. 
> min_hard_comp decides the threshold when silence injection actually happens, 
> and the default for that is 0.1, which is more than a frame, therefore not 
> acceptable if we want to maintain <1 frame accuracy. Or at least that is how 
> I think it should work.

I’ve found that aresample=async=1:min_hard_comp=0.01, as discussed here, works 
well to add audio samples to maintain timestamp accuracy when muxing into a 
format like mov. However, this approach doesn’t work if the sparseness of the 
audio stream is at the end of the stream. Is there a way to use min_hard_comp 
to consider differences between a timestamp and audio data when one of the ends 
of that range is the end of the file?
Best Regards,
Dave Rice

___
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/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Nov 14, 2020, at 7:14 PM, Marton Balint  wrote:
> 
> On Fri, 6 Nov 2020, Michael Niedermayer wrote:
> 
>> On Wed, Nov 04, 2020 at 10:44:56PM +0100, Marton Balint wrote:
>>> 
>>> 
>>> On Wed, 4 Nov 2020, Michael Niedermayer wrote:
>>> 
 we have "millisecond" based formats, rounded timestamps
 we have "exact" cases, maybe the timebase being 1 packet/frame per tick
 we have "high precission" where the timebase is so precisse it doesnt 
 matter
 
 This here though is a bit an oddball, the size if 1 PCM frame is 1 sample
 The timebase is not a millisecond based one, its not 1 frame either nor is
 it exact nor high precission.
 Its 1 video frame, and whatever amount of audio there is in the container
 
 which IIUC can differ from 1 video frame even rounded.
 maybe this just doesnt occur and each frame has a count of samples always
 rounded to the closes integer count for the video frame.
>>> 
>>> The difference between the audio timestamp and the video timestamp for
>>> packets from the same DV frame is at most 0.3929636797*frame_duration as the
>>> specification says, as Dave quoted, so I don't see how the error can be
>>> bigger than this.
>>> 
>>> It looks to me you are mixing timestamps coming from a demuxer, and
>>> timestamps you calculate by counting the number of demuxed/decoded audio
>>> samples or video frames. Synchronization is done using the former.
>>> 
>> 
 
 But if for example some hardware was using internally a 16 sample buffer
 and only put multiplies of 16 samples in frames this would introduce a
 considerable amount of jitter in the timestamps in relation to the actual
 duration. And using async to fix this without introducing more problems
 might require some care.
>>> 
>>> I still don't see why timestamp or duration jitter is a problem
>> 
>>> as long as
>>> the error is below frame_duration/2. You can safely use async with
>>> min_hard_comp set to frame_duration/2.
>> 
>> Thats exactly what i meant. an async like filter which behaves differently
>> or async with a different value there can mess this up.
>> IMHO such mess up is ok when the input is corrupted or invalid. OTOH
>> here it is valid and correct data.
>> 
>> 
>>> 
>>> In general, don't you find it problematic that the dv demuxer can return
>>> different timestamps if you read packets sequentially and if you seek to the
>>> end of a file? It looks like a huge bug
>> 
>> yes, this is not great
>> but even with your patch you still have this effect
>> when seeking to some point in time a player has to output video and
>> audio to the user at an exact time and that will differ even with async
>> from linear playbacks presentation
>> 
>> 
>>> which is not fixable if you insist
>>> on sample counting...
>> 
>> I think you misunderstood me, or maybe i didnt state my opinion well,
>> iam not saying that i consider what dv in git does good. Rather that there
>> is a problem beyond what these patches fix.
>> Some concept of timestamp accuracy independant of the distance of 
>> representable
>> values would be usefull.
>> if you take teh 1/25 or whatever they are based on dv timestamps and convert 
>> that
>> to teh mpeg 90khz based ones thats not making it that accurate.
>> OTOH if you take 1/25 based audio where each packet is 1/25sec worth of 
>> samples
>> that very well might be sample accurate or even beyond.
>> knowing this accuracy is usefull for configuring a async like filter or also 
>> in
>> knowing how to deal with inconsistencies, is that timestamp jtter ? or the 
>> sample
>> rate jittering / some droped samples ?
>> Its important to know as in one instance its the timestamps that need 
>> adjustment
>> while in the other the samples need adjustment
>> ATM its down to the user to figure out on a file by file base how to deal or
>> ignore this. Instead it should be possible for an automated system to
>> compensate such issues ...
> 
> OK, but the automated solution is far from trivial, e.g. it should start with 
> a analysis of the file to check if the sample rate is accurate or not... And 
> if it is not, is the difference constant througout the file? Then there are 
> several methods to fix it and the user might have a preference. E.g consider 
> audio clock "master" and duplicate/drop video frames. Or keep all video 
> frames, but stretch audio (with or without pitch correction - and which 
> filter you want for pitch correction? atempo? rubberband?). So making it 
> automated is not trivial at all.
> 
> Anyhow, is it OK to apply this patch then?

Nudging on this, as this patch has been very helpful to avoid tedious 
workarounds. I’d love to see it merged. If any further test results would be 
useful, let me know.
Dave

___
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 

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Nov 6, 2020, at 4:03 PM, Michael Niedermayer  
> wrote:
> 
> On Wed, Nov 04, 2020 at 10:44:56PM +0100, Marton Balint wrote:
>> 
>> On Wed, 4 Nov 2020, Michael Niedermayer wrote:
>> 
>>> we have "millisecond" based formats, rounded timestamps
>>> we have "exact" cases, maybe the timebase being 1 packet/frame per tick
>>> we have "high precission" where the timebase is so precisse it doesnt matter
>>> 
>>> This here though is a bit an oddball, the size if 1 PCM frame is 1 sample
>>> The timebase is not a millisecond based one, its not 1 frame either nor is
>>> it exact nor high precission.
>>> Its 1 video frame, and whatever amount of audio there is in the container
>>> 
>>> which IIUC can differ from 1 video frame even rounded.
>>> maybe this just doesnt occur and each frame has a count of samples always
>>> rounded to the closes integer count for the video frame.
>> 
>> The difference between the audio timestamp and the video timestamp for
>> packets from the same DV frame is at most 0.3929636797*frame_duration as the
>> specification says, as Dave quoted, so I don't see how the error can be
>> bigger than this.
>> 
>> It looks to me you are mixing timestamps coming from a demuxer, and
>> timestamps you calculate by counting the number of demuxed/decoded audio
>> samples or video frames. Synchronization is done using the former.
>> 
> 
>>> 
>>> But if for example some hardware was using internally a 16 sample buffer
>>> and only put multiplies of 16 samples in frames this would introduce a
>>> considerable amount of jitter in the timestamps in relation to the actual
>>> duration. And using async to fix this without introducing more problems
>>> might require some care.
>> 
>> I still don't see why timestamp or duration jitter is a problem 
> 
>> as long as
>> the error is below frame_duration/2. You can safely use async with
>> min_hard_comp set to frame_duration/2.
> 
> Thats exactly what i meant. an async like filter which behaves differently
> or async with a different value there can mess this up.
> IMHO such mess up is ok when the input is corrupted or invalid. OTOH
> here it is valid and correct data.
> 
>> In general, don't you find it problematic that the dv demuxer can return
>> different timestamps if you read packets sequentially and if you seek to the
>> end of a file? It looks like a huge bug
> 
> yes, this is not great
> but even with your patch you still have this effect
> when seeking to some point in time a player has to output video and
> audio to the user at an exact time and that will differ even with async
> from linear playbacks presentation

When trying to workaround the loss of audio sync, I use -skip_initial_bytes on 
the dv input to jump to the frame after a missing audio pack to read from that 
point to keep audio and video in sync from that offset in the bytestream (at 
least until the next missing audio source pack).

>> which is not fixable if you insist
>> on sample counting...
> 
> I think you misunderstood me, or maybe i didnt state my opinion well,
> iam not saying that i consider what dv in git does good. Rather that there
> is a problem beyond what these patches fix. 
> Some concept of timestamp accuracy independant of the distance of 
> representable
> values would be usefull.
> if you take teh 1/25 or whatever they are based on dv timestamps and convert 
> that
> to teh mpeg 90khz based ones thats not making it that accurate.
> OTOH if you take 1/25 based audio where each packet is 1/25sec worth of 
> samples
> that very well might be sample accurate or even beyond.
> knowing this accuracy is usefull for configuring a async like filter or also 
> in
> knowing how to deal with inconsistencies, is that timestamp jtter ? or the 
> sample
> rate jittering / some droped samples ? 
> Its important to know as in one instance its the timestamps that need 
> adjustment 
> while in the other the samples need adjustment
> ATM its down to the user to figure out on a file by file base how to deal or
> ignore this. Instead it should be possible for an automated system to 
> compensate such issues ...

As mentioned elsewhere, some automation (or at least a logged hint) would be 
helpful to add or suggest aresample=async=1 to fill the gaps when using 
containers that don’t support sparse audio. With Marton’s patch, the user has 
the opportunity to use that filter to keep the audio in sync.

[…]

Dave Rice

___
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/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Nov 1, 2020, at 3:58 PM, Marton Balint  wrote:
> 
> 
> 
> On Sun, 1 Nov 2020, Michael Niedermayer wrote:
> 
>> On Sat, Oct 31, 2020 at 05:56:24PM +0100, Marton Balint wrote:
>>> Fixes out of sync timestamps in ticket #8762.
>>> 
>>> Signed-off-by: Marton Balint 
>>> ---
>>> libavformat/dv.c   | 16 ++--
>>> tests/ref/seek/lavf-dv | 18 +-
>>> 2 files changed, 11 insertions(+), 23 deletions(-)
>>> 
>>> diff --git a/libavformat/dv.c b/libavformat/dv.c
>>> index 3e0d12c0e3..26a78139f5 100644
>>> --- a/libavformat/dv.c
>>> +++ b/libavformat/dv.c
>>> @@ -49,7 +49,6 @@ struct DVDemuxContext {
>>> uint8_t   audio_buf[4][8192];
>>> int   ach;
>>> int   frames;
>>> -uint64_t  abytes;
>>> };
>>> 
>>> static inline uint16_t dv_audio_12to16(uint16_t sample)
>>> @@ -258,7 +257,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, 
>>> const uint8_t *frame)
>>> c->ast[i] = avformat_new_stream(c->fctx, NULL);
>>> if (!c->ast[i])
>>> break;
>>> -avpriv_set_pts_info(c->ast[i], 64, 1, 3);
>>> +avpriv_set_pts_info(c->ast[i], 64, c->sys->time_base.num, 
>>> c->sys->time_base.den);
>>> c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
>>> c->ast[i]->codecpar->codec_id   = AV_CODEC_ID_PCM_S16LE;
>>> 
>>> @@ -387,8 +386,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, 
>>> AVPacket *pkt,
>>> for (i = 0; i < c->ach; i++) {
>>> c->audio_pkt[i].pos  = pos;
>>> c->audio_pkt[i].size = size;
>>> -c->audio_pkt[i].pts  = c->abytes * 3 * 8 /
>>> -   c->ast[i]->codecpar->bit_rate;
>>> +c->audio_pkt[i].pts  = (c->sys->height == 720) ? (c->frames & ~1) 
>>> : c->frames;
>>> ppcm[i] = c->audio_buf[i];
>>> }
>>> if (c->ach)
>>> @@ -401,10 +399,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, 
>>> AVPacket *pkt,
>>> c->audio_pkt[2].size = c->audio_pkt[3].size = 0;
>>> } else {
>>> c->audio_pkt[0].size = c->audio_pkt[1].size = 0;
>>> -c->abytes   += size;
>>> }
>>> -} else {
>>> -c->abytes += size;
>>> }
>>> 
>>> /* Now it's time to return video packet */
>> 
>> Please correct me if iam wrong but
>> in cases where no audio is missing or damaged, this would also ignore how 
>> much
>> audio is in each packet. So you could have lets say a timestamp difference
>> of excatly 1 second between 2 packets while their is actually not exactly
>> 1 second worth of audio samples between them.
> 
> This is true, by using the frame counter (and the video time base) for audio, 
> we lose some audio packet timestamp precision inherently. However I don't 
> consider this a problem, audio timestamps do not have to be sample accurate, 
> for most formats they are not. Also it is not practical to keep track of how 
> many samples are there in the packets, for example when you do seeking, 
> obviously you can't read all the audio data before the seek point to get a 
> precise sample accurate timestamp.

Good point.

> What matters is that based on what I understand about the DV format (but 
> maybe Dave can confirm or deny this) the divergence between the audio 
> timestamp and the video timestamp in a DV frame must be less than 1/3 frame 
> duration even for unlocked mode:
> 
> http://www.adamwilt.com/DV-FAQ-tech.html#LockedAudio

The divergence could be a little larger than 1/3 frame in unlocked mode. 
IEC61384-2 defines the allowable range of minimum to maximum samples per frame 
and the maximum allowable divergence of accumulated samples per frame.

Mode   | Min-Max   | Allowance of accumulated difference
NTSC 48000 | 1580-1620 | 20
NTSC 44100 | 1452-1489 | 19
NTSC 32000 | 1053-1080 | 14
PAL  48000 | 1896-1944 | 24
PAL  44100 | 1742-1786 | 22
PAL  32000 | 1264-1296 | 16

The divergence between the audio timestamp and video timestamp is conditional 
on the mode, so that would be

Mode   | Max divergence as percentage of frame duration
NTSC 48000 | 0.3742511235
NTSC 44100 | 0.3869807536
NTSC 32000 | 0.3929636797
PAL  48000 | 0.3125
PAL  44100 | 0.3117913832
PAL  32000 | 0.3125

0.3

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Oct 31, 2020, at 5:15 PM, Marton Balint  wrote:
> On Sat, 31 Oct 2020, Dave Rice wrote:
>>> On Oct 31, 2020, at 3:47 PM, Marton Balint  wrote:
>>> On Sat, 31 Oct 2020, Dave Rice wrote:
>>>> Hi Marton,
>>>>> On Oct 31, 2020, at 12:56 PM, Marton Balint  wrote:
>>>>> Fixes out of sync timestamps in ticket #8762.
>>>> Although Michael’s recent patch does address the issue documented in 8762, 
>>>> I haven’t found this patch to fix the issue. I tried with -c:a copy and 
>>>> with -c:a pcm_s16le with some sample files that exhibit this issue but 
>>>> each output was out of sync. I put an output at 
>>>> https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795. That 
>>>> output notes that 3597 packages of video are read and 3586 packets of 
>>>> audio. In the resulting file, at the end of the timeline the audio is 9 
>>>> frames out of sync and my output video stream is 00:02:00.020 and output 
>>>> audio stream is 00:01:59.653.
>>>> Beyond copying or encoding the audio, are there other options I should use 
>>>> to test this?
>>> Well, it depends on what you want. After this patch you should get a file 
>>> which has audio packets synced to video, but the audio stream is sparse, 
>>> not every video packet has a corresponding audio packet. (It looks like our 
>>> MOV muxer does not support muxing of sparse audio therefore does not 
>>> produce proper timestamps. But MKV does, please try that.)
>>> You can also make ffmpeg generate the missing audio based on packet 
>>> timestamps. Swresample has an async=1 option, so something like this should 
>>> get you synced audio with continous audio packets:
>>> ffmpeg -y -i 167052_12.dv -c:v copy \
>>> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov
>> 
>> Thank you for this. With the patch and async, the result is synced and the 
>> resulting audio was the same as Michael’s patch.
>> 
>> Could you explain why you used min_hard_comp here? IIUC min_hard_comp is a 
>> set a threshold between the strategies of trim/fill or stretch/squeeze to 
>> align the audio to time; however, the async documentation says "Setting this 
>> to 1 will enable filling and trimming, larger values represent the maximum 
>> amount in samples that the data may be stretched or squeezed” so I thought 
>> that async=1 would not permit stretch/squeeze anyway.
> 
> It is documented poorly, but if you check the source code you will see that 
> async=1 implicitly sets min_comp to 0.001 enabling trimming/dropping. 
> min_hard_comp decides the threshold when silence injection actually happens, 
> and the default for that is 0.1, which is more than a frame, therefore not 
> acceptable if we want to maintain <1 frame accuracy. Or at least that is how 
> I think it should work.

Thanks for the explanation.
I’ve tested this patch with larger sample sets and really appreciate that the 
audio timestamps are now properly ordered. For instance with 
https://archive.org/download/dvr-007/DVR_007.dv, before the patch the 
timestamps for audio around 20 - 30 seconds get severely jumbled which makes it 
difficult to use astats metadata. While the documentation could be better, as 
I’ve seen many forums entries of ffmpeg users struggling to keep dv from tape 
transfers in sync, I don’t think that’s blocking. I’d really like to see this 
merged.

Although this patch resolves the issue, I think Michael’s patch is interesting 
too as it would allow access to the audio dif blocks of dv frames with no audio 
source pack metadata. I’d have to test more to find an instance where the 
'dvaudio_concealment pass’ option of Michael’s patch provides a different 
result than the added silence from async.

Thanks so much.
Dave

> Regards,
> Marton
> 
>>>>> Signed-off-by: Marton Balint 
>>>>> ---
>>>>> libavformat/dv.c   | 16 ++--
>>>>> tests/ref/seek/lavf-dv | 18 +-
>>>>> 2 files changed, 11 insertions(+), 23 deletions(-)
>>>>> diff --git a/libavformat/dv.c b/libavformat/dv.c
>>>>> index 3e0d12c0e3..26a78139f5 100644
>>>>> --- a/libavformat/dv.c
>>>>> +++ b/libavformat/dv.c
>>>>> @@ -49,7 +49,6 @@ struct DVDemuxContext {
>>>>>   uint8_t   audio_buf[4][8192];
>>>>>   int   ach;
>>>>>   int   frames;
>>>>> -uint64_t  abytes;
>>>>> };
>>>>> static inline uint16_t dv_audio_12to16(uint16_t 

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

> On Oct 31, 2020, at 3:47 PM, Marton Balint  wrote:
> On Sat, 31 Oct 2020, Dave Rice wrote:
> 
>> Hi Marton,
>> 
>>> On Oct 31, 2020, at 12:56 PM, Marton Balint  wrote:
>>> Fixes out of sync timestamps in ticket #8762.
>> 
>> Although Michael’s recent patch does address the issue documented in 8762, I 
>> haven’t found this patch to fix the issue. I tried with -c:a copy and with 
>> -c:a pcm_s16le with some sample files that exhibit this issue but each 
>> output was out of sync. I put an output at 
>> https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795. That 
>> output notes that 3597 packages of video are read and 3586 packets of audio. 
>> In the resulting file, at the end of the timeline the audio is 9 frames out 
>> of sync and my output video stream is 00:02:00.020 and output audio stream 
>> is 00:01:59.653.
>> 
>> Beyond copying or encoding the audio, are there other options I should use 
>> to test this?
> 
> Well, it depends on what you want. After this patch you should get a file 
> which has audio packets synced to video, but the audio stream is sparse, not 
> every video packet has a corresponding audio packet. (It looks like our MOV 
> muxer does not support muxing of sparse audio therefore does not produce 
> proper timestamps. But MKV does, please try that.)
> 
> You can also make ffmpeg generate the missing audio based on packet 
> timestamps. Swresample has an async=1 option, so something like this should 
> get you synced audio with continous audio packets:
> ffmpeg -y -i 167052_12.dv -c:v copy \
> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov

Thank you for this. With the patch and async, the result is synced and the 
resulting audio was the same as Michael’s patch.

Could you explain why you used min_hard_comp here? IIUC min_hard_comp is a set 
a threshold between the strategies of trim/fill or stretch/squeeze to align the 
audio to time; however, the async documentation says "Setting this to 1 will 
enable filling and trimming, larger values represent the maximum amount in 
samples that the data may be stretched or squeezed” so I thought that async=1 
would not permit stretch/squeeze anyway.

> Regards,
> Marton
> 
> 
>> 
>>> Signed-off-by: Marton Balint 
>>> ---
>>> libavformat/dv.c   | 16 ++--
>>> tests/ref/seek/lavf-dv | 18 +-
>>> 2 files changed, 11 insertions(+), 23 deletions(-)
>>> diff --git a/libavformat/dv.c b/libavformat/dv.c
>>> index 3e0d12c0e3..26a78139f5 100644
>>> --- a/libavformat/dv.c
>>> +++ b/libavformat/dv.c
>>> @@ -49,7 +49,6 @@ struct DVDemuxContext {
>>>uint8_t   audio_buf[4][8192];
>>>int   ach;
>>>int   frames;
>>> -uint64_t  abytes;
>>> };
>>> static inline uint16_t dv_audio_12to16(uint16_t sample)
>>> @@ -258,7 +257,7 @@ static int dv_extract_audio_info(DVDemuxContext *c, 
>>> const uint8_t *frame)
>>>c->ast[i] = avformat_new_stream(c->fctx, NULL);
>>>if (!c->ast[i])
>>>break;
>>> -avpriv_set_pts_info(c->ast[i], 64, 1, 3);
>>> +avpriv_set_pts_info(c->ast[i], 64, c->sys->time_base.num, 
>>> c->sys->time_base.den);
>>>c->ast[i]->codecpar->codec_type = AVMEDIA_TYPE_AUDIO;
>>>c->ast[i]->codecpar->codec_id   = AV_CODEC_ID_PCM_S16LE;
>>> @@ -387,8 +386,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, 
>>> AVPacket *pkt,
>>>for (i = 0; i < c->ach; i++) {
>>>c->audio_pkt[i].pos  = pos;
>>>c->audio_pkt[i].size = size;
>>> -c->audio_pkt[i].pts  = c->abytes * 3 * 8 /
>>> -   c->ast[i]->codecpar->bit_rate;
>>> +c->audio_pkt[i].pts  = (c->sys->height == 720) ? (c->frames & ~1) 
>>> : c->frames;
>>>ppcm[i] = c->audio_buf[i];
>>>}
>>>if (c->ach)
>>> @@ -401,10 +399,7 @@ int avpriv_dv_produce_packet(DVDemuxContext *c, 
>>> AVPacket *pkt,
>>>c->audio_pkt[2].size = c->audio_pkt[3].size = 0;
>>>} else {
>>>c->audio_pkt[0].size = c->audio_pkt[1].size = 0;
>>> -c->abytes   += size;
>>>}
>>> -} else {
>>> -c->abytes += size;
>>>}
>>> 
>>>/* Now it's time to retur

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> -ret: 0 st: 1 flags:0  ts:-0.058333
> +ret: 0 st: 1 flags:0  ts:-0.04
> ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:  0 
> size:144000
> -ret: 0 st: 1 flags:1  ts: 2.835833
> +ret: 0 st: 1 flags:1  ts: 2.84
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> ret: 0 st:-1 flags:0  ts: 1.730004
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> @@ -31,10 +31,10 @@ ret: 0 st: 0 flags:0  ts:-0.48
> ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:  0 
> size:144000
> ret: 0 st: 0 flags:1  ts: 2.40
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> -ret: 0 st: 1 flags:0  ts: 1.306667
> -ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> -ret: 0 st: 1 flags:1  ts: 0.200833
> +ret: 0 st: 1 flags:0  ts: 1.32
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> +ret: 0 st: 1 flags:1  ts: 0.20
> +ret: 0 st: 0 flags:1 dts: 0.20 pts: 0.20 pos: 72 
> size:144000
> ret: 0 st:-1 flags:0  ts:-0.904994
> ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:  0 
> size:144000
> ret: 0 st:-1 flags:1  ts: 1.989173
> @@ -43,9 +43,9 @@ ret: 0 st: 0 flags:0  ts: 0.88
> ret: 0 st: 0 flags:1 dts: 0.88 pts: 0.88 pos:3168000 
> size:144000
> ret: 0 st: 0 flags:1  ts:-0.24
> ret: 0 st: 0 flags:1 dts: 0.00 pts: 0.00 pos:  0 
> size:144000
> -ret: 0 st: 1 flags:0  ts: 2.671667
> +ret: 0 st: 1 flags:0  ts: 2.68
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> -ret: 0 st: 1 flags:1  ts: 1.565833
> +ret: 0 st: 1 flags:1  ts: 1.56
> ret: 0 st: 0 flags:1 dts: 0.96 pts: 0.96 pos:3456000 
> size:144000
> ret: 0 st:-1 flags:0  ts: 0.460008
> ret: 0 st: 0 flags:1 dts: 0.48 pts: 0.48 pos:1728000 
> size:144000
> — 
> 2.26.2

Best Regards,
Dave Rice

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

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/dv: allow returning damaged audio

[Dave Rice]

> On Aug 3, 2020, at 5:16 PM, Michael Niedermayer  
> wrote:
> 
> On Mon, Aug 03, 2020 at 10:38:21PM +0200, Marton Balint wrote:
>> 
>> 
>> On Sun, 2 Aug 2020, Dave Rice wrote:
>> 
>>> 
>>> 
>>>> On Aug 1, 2020, at 5:26 PM, Marton Balint  wrote:
>>>> 
>>>> 
>>>> 
>>>> On Sat, 1 Aug 2020, Michael Niedermayer wrote:
>>>> 
>>>>> On Sat, Aug 01, 2020 at 07:28:53PM +0200, Marton Balint wrote:
>>>>>> 
>>>>>> 
>>>>>> On Sat, 1 Aug 2020, Michael Niedermayer wrote:
>>>>>> 
>>>>>>> Fixes: Ticket8762
>>>>>>> Signed-off-by: Michael Niedermayer 
>>>>>>> ---
>>>>>>> libavformat/dv.c | 49 +---
>>>>>>> 1 file changed, 42 insertions(+), 7 deletions(-)
>>>>>> 
>>>>>> If "dv remux loses sync", then the timestamps should be fixed, not
>>>>>> additional packets should be generated based on previously read packet 
>>>>>> data
>>>>>> (which is a fragile approach to begin with, e.g. what if the first frame 
>>>>>> is
>>>>>> the corrupt one?).
>>>>> 
>>>>> Ticket8762 is about stream copy, so if no packets are returned for audio
>>>>> but are for video and just timestamps are updated this would at least on
>>>>> its own probably not work that well.
>>>> 
>>>> If the timestamps are good, a good player should be able to play it
>>>> correctly, even if audio stream is sparse.
>>>> 
>>>> None of the demuxers generate packets because the timestamps are not
>>>> continous, I just don't think it would be consistent if DV suddenly
>>>> started to do this. E.g. what if the user wants to drop video with
>>>> no audio?
>>> 
>>> In practice, when dv frames with video and no audio are interleaved
>>> within a dv stream that otherwise has both, it is because the playback
>>> videotape player of the dv tape is in pause mode or the tape is damaged.
>>> These frames most common are filled with only video dif blocks that note
>>> concealment (so the image is a copy of a prior image) and the audio
>>> source pack metadata is missing, but the paylock of the audio dif blocks
>>> are filled with error code so they would decode as silence.
>> 
>> But if the audio source pack metadata is missing, then how can you determine
>> the audio settings?

I tested with QuickTime Player 7 and when frames are read with the audio source 
pack metadata missing, the first audio source pack is used. So these frames 
provide silent output as an earlier audio source pack is used. The disadvantage 
here is that a mid stream change such as 32kHz to 48kHz causes QuickTime Player 
7 to mangle the audio by applying the wrong characteristics.

>> Or the number of samples the errornous frame contains
>> (e.g. 1600 v.s 1602)?
> 
> some testcase would be useful here where this is done clearly wrong currently

I put two additional samples at 
https://archive.org/download/001.dv.audiogap/001.dv.audiogap.dv 
<https://archive.org/download/001.dv.audiogap/001.dv.audiogap.dv> and 
https://archive.org/download/001.dv.audiogap/DVC00036_001.dv.audiogap.dv 
<https://archive.org/download/001.dv.audiogap/DVC00036_001.dv.audiogap.dv>. 
Each contains a series of frames in the middle that have all video blocks as 
concealed and all audio blocks are simply error code with no audio source pack.

For each example, both "ffmpeg -i file -c copy out” and “ffmpeg -i file out” 
has a loss of sync in the result and an audio track shorter than the video.

But true, a frame with no audio source pack does not communicate if it should 
be 1600 or 1602 samples.

In the SMPTE specification for DV at 
http://web.archive.org/web/20060927044735/http://www.smpte.org/smpte_store/standards/pdf/s314m.pdf
 
<http://web.archive.org/web/20060927044735/http://www.smpte.org/smpte_store/standards/pdf/s314m.pdf>,
 it says on page 18 that for NTSC systems, the five-frame pattern should be: 
1600, 1602, 1602, 1602, 1602. So if a frame has no audio source pack, the 
pattern of prior frames could be used or simply use this pattern upon finding a 
sequence of such frames starting at 1600. Or possibly the relationship between 
the starting time of the audio data and the starting time for the video data 
could be used to guess if 1600 or 1602 maintains the alignment more closely.

>> Also maybe setting the CORRUPT packet flag should be done in this case?
> 
> yes was thinking that too, that should be in the next revision

In the reference specification, table 26 shows how the STA value is interpreted 
to note if the frame contains concealed video DIF blocks or not. This doesn’t 
necessarily mean that the frame is corrupt, but that it is the product of data 
concealment caused by a misreading of the DV videotape.

[…]
Dave


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

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/dv: allow returning damaged audio

[Dave Rice]

> On Aug 1, 2020, at 5:26 PM, Marton Balint  wrote:
> 
> 
> 
> On Sat, 1 Aug 2020, Michael Niedermayer wrote:
> 
>> On Sat, Aug 01, 2020 at 07:28:53PM +0200, Marton Balint wrote:
>>> 
>>> 
>>> On Sat, 1 Aug 2020, Michael Niedermayer wrote:
>>> 
 Fixes: Ticket8762
 Signed-off-by: Michael Niedermayer 
 ---
 libavformat/dv.c | 49 +---
 1 file changed, 42 insertions(+), 7 deletions(-)
>>> 
>>> If "dv remux loses sync", then the timestamps should be fixed, not
>>> additional packets should be generated based on previously read packet data
>>> (which is a fragile approach to begin with, e.g. what if the first frame is
>>> the corrupt one?).
>> 
>> Ticket8762 is about stream copy, so if no packets are returned for audio
>> but are for video and just timestamps are updated this would at least on
>> its own probably not work that well.
> 
> If the timestamps are good, a good player should be able to play it 
> correctly, even if audio stream is sparse.
> 
> None of the demuxers generate packets because the timestamps are not 
> continous, I just don't think it would be consistent if DV suddenly started 
> to do this. E.g. what if the user wants to drop video with no audio?

In practice, when dv frames with video and no audio are interleaved within a dv 
stream that otherwise has both, it is because the playback videotape player of 
the dv tape is in pause mode or the tape is damaged. These frames most common 
are filled with only video dif blocks that note concealment (so the image is a 
copy of a prior image) and the audio source pack metadata is missing, but the 
paylock of the audio dif blocks are filled with error code so they would decode 
as silence.

I did a test of 114 dv tape transfers
61 no difference in demuxing between pass and drop (using this patch)
31 the difference is only in the final frames so the impact of the 
demuxer option would be hard for the user to see, no loss of sync, just video 
with no audio at the end
22 the difference occurs mid-stream, with the drop option the demuxer 
outputs video and audio at different rate when hitting frames with no audio 
source pack, so the output of the demuxer loses sync

>> about the case of a damaged first frame. Do you have a testcase ?
> 
> No, but it can happen, can't it? If the stream starts with no audio for 1 
> second you will have 1 second A-V desync, as far as I see, that is why I 
> believe fixing the timestamps is the proper fix.

Yes this happens (though it is more rare and didn’t occur in the test noted 
above). In that case, ffmpeg shows no audio at all and I’d have to read the 
stream at later frames using -skip_initial_bytes.

> 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".

___
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/mov: fix memleak

[Dave Rice]

Hi Andreas,

> On Jun 27, 2020, at 5:09 PM, Andreas Rheinhardt 
>  wrote:
> 
> Zhao Zhili:
>> Remove the check on dv_demux since dv_fctx will leak if allocate
>> dv_demux failed.
>> ---
>> libavformat/mov.c | 7 +++
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>> 
>> diff --git a/libavformat/mov.c b/libavformat/mov.c
>> index adc52de947..f179b6efdd 100644
>> --- a/libavformat/mov.c
>> +++ b/libavformat/mov.c
>> @@ -7357,10 +7357,9 @@ static int mov_read_close(AVFormatContext *s)
>> av_freep(>coll);
>> }
>> 
>> -if (mov->dv_demux) {
>> -avformat_free_context(mov->dv_fctx);
>> -mov->dv_fctx = NULL;
>> -}
>> +av_freep(>dv_demux);
>> +avformat_free_context(mov->dv_fctx);
>> +mov->dv_fctx = NULL;
>> 
>> if (mov->meta_keys) {
>> for (i = 1; i < mov->meta_keys_count; i++) {
>> 
> Do you have a sample for this? I am asking because there are more
> memleaks related to dv audio and I have a patch [1] for them, but I
> never found any sample for dv in mov/mp4, so it was never applied.

I’m working with a lot of DV lately. Could you clarify what type of sample that 
you are looking for?

> If I am not mistaken, then you are not only fixing a leak of dv_fctx in
> case allocation of dv_demux failed; you are also fixing a leak of
> dv_demux itself in the ordinary case where it could be successfully
> allocated. This should be reflected in the commit message.
> 
> - Andreas
> 
> [1]:
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20190916155502.17579-3-andreas.rheinha...@gmail.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 v1] avfilter: add colorstats, colorrgbstats, coloryuvstats video filter

[Dave Rice]

> On Dec 27, 2019, at 10:49 AM, Paul B Mahol  wrote:
> 
> That is because signalstats is doing more stuff.

signalstats includes options to disable some of the calculations, possibly this 
could be extended to enable or disable the ones you want. It would be 
interesting to merge these ideas rather than have two filters with such a 
substantial overlap.

> On 12/27/19, Limin Wang  wrote:
>> On Fri, Dec 27, 2019 at 03:20:19PM +0100, Paul B Mahol wrote:
>>> On 12/27/19, Limin Wang  wrote:
 On Fri, Dec 27, 2019 at 12:35:25PM +0100, Paul B Mahol wrote:
> You are duplicating some functionality of signalstats filter.
> 
 Yes, I have other function need to use the mean and stdev which is
 support in showinfo filter(only 8bit and don't support packed format,
 no multi-thread), and signalstats don't support rgb format and don't
 have stdev, also it have too many other function and difficult to change
 it, so I think it's more simple to create a new filter to do it.
 
>>> 
>>> No, unacceptable. use signalstats filter.
>> 
>> The performance is one major reason also, below is the profiling result for
>> performance:
>> 
>> ./ffmpeg -nostats -f lavfi -i testsrc2=4k:d=2 -vf
>> bench=start,signalstats,bench=stop -f null -
>> [bench @ 0x3fb9080] t:0.161589 avg:0.165756 max:0.169923 min:0.161589
>> [bench @ 0x3fb9080] t:0.160334 avg:0.163948 max:0.169923 min:0.160334
>> [bench @ 0x3fb9080] t:0.160345 avg:0.163047 max:0.169923 min:0.160334
>> [bench @ 0x3fb9080] t:0.160924 avg:0.162623 max:0.169923 min:0.160334
>> [bench @ 0x3fb9080] t:0.160318 avg:0.162238 max:0.169923 min:0.160318
>> 
>> ./ffmpeg -nostats -f lavfi -i testsrc2=4k:d=2 -vf
>> bench=start,colorstats,bench=stop -f null -
>> [bench @ 0x26f6100] t:0.012596 avg:0.012612 max:0.012628 min:0.012596
>> [bench @ 0x26f6100] t:0.012542 avg:0.012588 max:0.012628 min:0.012542
>> [bench @ 0x26f6100] t:0.012529 avg:0.012573 max:0.012628 min:0.012529
>> [bench @ 0x26f6100] t:0.012532 avg:0.012565 max:0.012628 min:0.012529
>> [bench @ 0x26f6100] t:0.012527 avg:0.012559 max:0.012628 min:0.012527
>> [bench @ 0x26f6100] t:0.012525 avg:0.012554 max:0.012628 min:0.012525
>> [bench @ 0x26f6100] t:0.012522 avg:0.012550 max:0.012628 min:0.012522
>> [bench @ 0x26f6100] t:0.012552 avg:0.012550 max:0.012628 min:0.012522
>> 
>> 
>>> 
 
> On 12/27/19, lance.lmw...@gmail.com  wrote:
>> From: Limin Wang 
>> 
>> Signed-off-by: Limin Wang 
>> ---
>> doc/filters.texi|  74 ++
>> libavfilter/Makefile|   1 +
>> libavfilter/allfilters.c|   3 +
>> libavfilter/vf_colorstats.c | 461
>> 
>> 4 files changed, 539 insertions(+)
>> create mode 100644 libavfilter/vf_colorstats.c
>> 
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index 8c5d3a5760..81968b2c17 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -7695,6 +7695,80 @@ For example to convert the input to
>> SMPTE-240M,
>> use
>> the command:
>> colorspace=smpte240m
>> @end example
>> 
>> +@section colorstats, colorrgbstats, coloryuvstats
>> +The filter provides statistical video measurements such as mean,
>> minimum,
>> maximum and
>> +standard deviation for each frame. The user can check for
>> unexpected/accidental errors
>> +very quickly with them.
>> +
>> +@var{colorrgbstats} report the color stats for RGB input video,
>> @var{coloryuvstats}
>> +to an YUV input video.
>> +
>> +These filters accept the following parameters:
>> +@table @option
>> +@item planes
>> +Set which planes to filter. Default is only the first plane.
>> +@end table
>> +
>> +By default the filter will report these metadata values if the
>> planes
>> +are processed:
>> +
>> +@table @option
>> +@item min.y, min.u, min.v, min.r, min.g, min.b, min.a
>> +Display the minimal Y/U/V/R/G/B/A plane value contained within the
>> input
>> frame.
>> +Expressed in range of [0, 1<> +
>> +@item pmin.y, pmin.u, pmin.v, pmin.r, pmin.g, pmin.b, min.a
>> +Display the minimal Y/U/V/R/G/B/A plane percentage of maximum
>> contained
>> within
>> +the input frame. Expressed in range of [0, 1]
>> +
>> +@item max.y, max.u, max.v, max.r, max.g, max.b, max.a
>> +Display the maximum Y/U/V/R/G/B/A plane value contained within the
>> input
>> frame.
>> +Expressed in range of [0, 1<> +
>> +@item pmax.y, pmax.u, pmax.v, pmax.r, pmax.g, pmax.b, pmax.a
>> +Display the maximum Y/U/V/R/G/B/A plane percentage of maximum
>> contained
>> within
>> +the input frame. Expressed in range of [0, 1]
>> +
>> +@item mean.y, mean.u, mean.v, mean.r, mean.g, mean.b, mean.a
>> +Display the Y/U/V/R/G/B/A plane mean value contained within the
>> input
>> frame.
>> +Expressed in range of [0, 1<> +

Re: [FFmpeg-devel] [PATCH] avformat/movenc: always write a colr atom

[Dave Rice]

Hi James,

> On Jul 16, 2019, at 9:47 AM, James Almer  wrote:
> 
> -{ "write_colr", "Write colr atom (Experimental, may be renamed or 
> changed, do not use from scripts)", 0, AV_OPT_TYPE_CONST, {.i64 = 
> FF_MOV_FLAG_WRITE_COLR}, INT_MIN, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 
> "movflags" },

The comment said specifically not to use this option in scripts; however, there 
were ffmpeg users (I admit myself as one) who wanted both to use scripts and 
write valid QuickTime files with colr data. I’d suggest permitting the option 
to remain but to change the default behavior from disabled to enabled. If I 
understand correctly, after this patch, encoding with `-movflags write_colr` 
would cause ffmpeg to fail with an error from the mov muxer.

Dave

___
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] [PATCHv3 1/2] ffplay: options to specify window position

[Dave Rice]

From caa816d70e69f85d49556ff341addab24ebcd942 Mon Sep 17 00:00:00 2001
From: Dave Rice 
Date: Mon, 1 Oct 2018 17:07:44 -0400
Subject: [PATCH 1/2] ffplay: options to specify window position

---
 doc/ffplay.texi  | 4 
 fftools/ffplay.c | 6 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/doc/ffplay.texi b/doc/ffplay.texi
index dcb86ce13c..99e1d7468a 100644
--- a/doc/ffplay.texi
+++ b/doc/ffplay.texi
@@ -74,6 +74,10 @@ as 100.
 Force format.
 @item -window_title @var{title}
 Set window title (default is the input filename).
+@item -left @var{title}
+Set the x position for the left of the window (default is a centered window).
+@item -top @var{title}
+Set the y position for the top of the window (default is a centered window).
 @item -loop @var{number}
 Loops movie playback  times. 0 means forever.
 @item -showmode @var{mode}
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index e375a32ec2..ab1f9faccf 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -314,6 +314,8 @@ static int default_width  = 640;
 static int default_height = 480;
 static int screen_width  = 0;
 static int screen_height = 0;
+static int screen_left = SDL_WINDOWPOS_CENTERED;
+static int screen_top = SDL_WINDOWPOS_CENTERED;
 static int audio_disable;
 static int video_disable;
 static int subtitle_disable;
@@ -1346,7 +1348,7 @@ static int video_open(VideoState *is)
 SDL_SetWindowTitle(window, window_title);
 
 SDL_SetWindowSize(window, w, h);
-SDL_SetWindowPosition(window, SDL_WINDOWPOS_CENTERED, 
SDL_WINDOWPOS_CENTERED);
+SDL_SetWindowPosition(window, screen_left, screen_top);
 if (is_full_screen)
 SDL_SetWindowFullscreen(window, SDL_WINDOW_FULLSCREEN_DESKTOP);
 SDL_ShowWindow(window);
@@ -3602,6 +3604,8 @@ static const OptionDef options[] = {
 { "framedrop", OPT_BOOL | OPT_EXPERT, {  }, "drop frames when 
cpu is too slow", "" },
 { "infbuf", OPT_BOOL | OPT_EXPERT, { _buffer }, "don't limit the 
input buffer size (useful with realtime streams)", "" },
 { "window_title", OPT_STRING | HAS_ARG, { _title }, "set window 
title", "window title" },
+{ "left", OPT_INT | HAS_ARG | OPT_EXPERT, { _left }, "set the x 
position for the left of the window", "x pos" },
+{ "top", OPT_INT | HAS_ARG | OPT_EXPERT, { _top }, "set the y 
position for the top of the window", "y pos" },
 #if CONFIG_AVFILTER
 { "vf", OPT_EXPERT | HAS_ARG, { .func_arg = opt_add_vfilter }, "set video 
filters", "filter_graph" },
 { "af", OPT_STRING | HAS_ARG, {  }, "set audio filters", 
"filter_graph" },
-- 
2.19.0

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

Re: [FFmpeg-devel] [PATCH 2/2] avdevice/sdl2 : add option to set window position

[Dave Rice]

> On Oct 2, 2018, at 1:32 AM, Gyan  wrote:
> 
> On Tue, Oct 2, 2018 at 2:47 AM Dave Rice  wrote:
> 
>> Allows arrangement of multiple windows such as:
>> ffmpeg -re -f lavfi -i mandelbrot -f sdl -window_x 1 -window_y 1
>> mandelbrot -vf waveform,format=yuv420p -f sdl -window_x 641 -window_y 1
>> waveform -vf vectorscope,format=yuv420p -f sdl -window_x 1 -window_y 481
>> vectorscop
>> 
>> From 00438983c96b5db227b9975a2c160fc6aac5219d Mon Sep 17 00:00:00 2001
>> From: Dave Rice 
>> Date: Mon, 1 Oct 2018 17:08:35 -0400
>> Subject: [PATCH 2/2] avdevice/sdl2 : add option to set window position
>> 
>> +if (!sdl->window_x)
>> +sdl->window_x = SDL_WINDOWPOS_CENTERED;
>> +if (!sdl->window_y)
>> +sdl->window_y = SDL_WINDOWPOS_CENTERED;
>> +SDL_SetWindowPosition(sdl->window, sdl->window_x, sdl->window_y);
>> 
> 
> What happens if the user value implies fully or partially out-of-canvas
> rendering?

I attempted to add an error message but am uncertain how to access the width 
and height of the canvas used. Any advice?

> For the former case, we may want to print a warning and reposition the
> window.
> If a partial window is drawable, then negative values can be valid and the
> lower range bound should be INT_MIN
> Also, the user can't position a window at top-left (0,0), so the default
> should probably be INT_MAX.
> 
> Gyan
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCHv2 1/2] ffplay: options to specify window position

[Dave Rice]

Thanks Marton for comments. Here is a revision to the first patch.

From 3fe6a9e5279a280af9a06843621737ddc44529cc Mon Sep 17 00:00:00 2001
From: Dave Rice 
Date: Mon, 1 Oct 2018 17:07:44 -0400
Subject: [PATCHv2 1/2] ffplay: options to specify window position

---
 doc/ffplay.texi  | 4 
 fftools/ffplay.c | 6 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/doc/ffplay.texi b/doc/ffplay.texi
index dcb86ce13c..a3da2cd570 100644
--- a/doc/ffplay.texi
+++ b/doc/ffplay.texi
@@ -74,6 +74,10 @@ as 100.
 Force format.
 @item -window_title @var{title}
 Set window title (default is the input filename).
+@item -screen_left @var{title}
+Set the x position for the left of the window (default is a centered window).
+@item -screen_top @var{title}
+Set the y position for the top of the window (default is a centered window).
 @item -loop @var{number}
 Loops movie playback  times. 0 means forever.
 @item -showmode @var{mode}
diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index e375a32ec2..6cc59b4d33 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -314,6 +314,8 @@ static int default_width  = 640;
 static int default_height = 480;
 static int screen_width  = 0;
 static int screen_height = 0;
+static int left = SDL_WINDOWPOS_CENTERED;
+static int top = SDL_WINDOWPOS_CENTERED;
 static int audio_disable;
 static int video_disable;
 static int subtitle_disable;
@@ -1346,7 +1348,7 @@ static int video_open(VideoState *is)
 SDL_SetWindowTitle(window, window_title);
 
 SDL_SetWindowSize(window, w, h);
-SDL_SetWindowPosition(window, SDL_WINDOWPOS_CENTERED, 
SDL_WINDOWPOS_CENTERED);
+SDL_SetWindowPosition(window, left, top);
 if (is_full_screen)
 SDL_SetWindowFullscreen(window, SDL_WINDOW_FULLSCREEN_DESKTOP);
 SDL_ShowWindow(window);
@@ -3602,6 +3604,8 @@ static const OptionDef options[] = {
 { "framedrop", OPT_BOOL | OPT_EXPERT, {  }, "drop frames when 
cpu is too slow", "" },
 { "infbuf", OPT_BOOL | OPT_EXPERT, { _buffer }, "don't limit the 
input buffer size (useful with realtime streams)", "" },
 { "window_title", OPT_STRING | HAS_ARG, { _title }, "set window 
title", "window title" },
+{ "left", OPT_INT | HAS_ARG | OPT_EXPERT, {  }, "set the x position 
for the left of the window", "x pos" },
+{ "top", OPT_INT | HAS_ARG | OPT_EXPERT, {  }, "set the y position for 
the top of the window", "y pos" },
 #if CONFIG_AVFILTER
 { "vf", OPT_EXPERT | HAS_ARG, { .func_arg = opt_add_vfilter }, "set video 
filters", "filter_graph" },
 { "af", OPT_STRING | HAS_ARG, {  }, "set audio filters", 
"filter_graph" },
-- 
2.19.0

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

[FFmpeg-devel] [PATCH 2/2] avdevice/sdl2 : add option to set window position

[Dave Rice]

Allows arrangement of multiple windows such as:
ffmpeg -re -f lavfi -i mandelbrot -f sdl -window_x 1 -window_y 1 mandelbrot -vf 
waveform,format=yuv420p -f sdl -window_x 641 -window_y 1 waveform -vf 
vectorscope,format=yuv420p -f sdl -window_x 1 -window_y 481 vectorscop

From 00438983c96b5db227b9975a2c160fc6aac5219d Mon Sep 17 00:00:00 2001
From: Dave Rice 
Date: Mon, 1 Oct 2018 17:08:35 -0400
Subject: [PATCH 2/2] avdevice/sdl2 : add option to set window position

---
 libavdevice/sdl2.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libavdevice/sdl2.c b/libavdevice/sdl2.c
index da5143078e..69c541da23 100644
--- a/libavdevice/sdl2.c
+++ b/libavdevice/sdl2.c
@@ -40,6 +40,7 @@ typedef struct {
 SDL_Renderer *renderer;
 char *window_title;
 int window_width, window_height;  /**< size of the window */
+int window_x, window_y;   /**< position of the window */
 int window_fullscreen;
 int window_borderless;
 int enable_quit_action;
@@ -217,6 +218,12 @@ static int sdl2_write_header(AVFormatContext *s)
 
 SDL_SetWindowTitle(sdl->window, sdl->window_title);
 
+if (!sdl->window_x)
+sdl->window_x = SDL_WINDOWPOS_CENTERED;
+if (!sdl->window_y)
+sdl->window_y = SDL_WINDOWPOS_CENTERED;
+SDL_SetWindowPosition(sdl->window, sdl->window_x, sdl->window_y);
+
 sdl->texture = SDL_CreateTexture(sdl->renderer, sdl->texture_fmt, 
SDL_TEXTUREACCESS_STREAMING,
  codecpar->width, codecpar->height);
 
@@ -337,6 +344,8 @@ static int sdl2_write_packet(AVFormatContext *s, AVPacket 
*pkt)
 static const AVOption options[] = {
 { "window_title",  "set SDL window title",   OFFSET(window_title), 
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, AV_OPT_FLAG_ENCODING_PARAM },
 { "window_size",   "set SDL window forced size", OFFSET(window_width), 
AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, AV_OPT_FLAG_ENCODING_PARAM },
+{ "window_x",  "set SDL window x position",  OFFSET(window_x), 
AV_OPT_TYPE_INT,{ .i64 = 0 },0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM 
},
+{ "window_y",  "set SDL window y position",  OFFSET(window_y), 
AV_OPT_TYPE_INT,{ .i64 = 0 },0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM 
},
 { "window_fullscreen", "set SDL window fullscreen",  
OFFSET(window_fullscreen), AV_OPT_TYPE_BOOL,  { .i64 = 0 },0, 1, 
AV_OPT_FLAG_ENCODING_PARAM },
 { "window_borderless", "set SDL window border off",  
OFFSET(window_borderless), AV_OPT_TYPE_BOOL,  { .i64 = 0 },0, 1, 
AV_OPT_FLAG_ENCODING_PARAM },
 { "window_enable_quit", "set if quit action is available", 
OFFSET(enable_quit_action), AV_OPT_TYPE_INT, {.i64=1},   0, 1, 
AV_OPT_FLAG_ENCODING_PARAM },
-- 
2.19.0

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

[FFmpeg-devel] [PATCH 1/2] ffplay: options to specify window position

[Dave Rice]

From 14d6833b564bd672613d50ecc4c3ede1768eee37 Mon Sep 17 00:00:00 2001
From: Dave Rice 
Date: Mon, 1 Oct 2018 17:07:44 -0400
Subject: [PATCH 1/2] ffplay: options to specify window position

---
 fftools/ffplay.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index e375a32ec2..e1ec2e9df2 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -314,6 +314,8 @@ static int default_width  = 640;
 static int default_height = 480;
 static int screen_width  = 0;
 static int screen_height = 0;
+static int window_x;
+static int window_y;
 static int audio_disable;
 static int video_disable;
 static int subtitle_disable;
@@ -1346,7 +1348,11 @@ static int video_open(VideoState *is)
 SDL_SetWindowTitle(window, window_title);
 
 SDL_SetWindowSize(window, w, h);
-SDL_SetWindowPosition(window, SDL_WINDOWPOS_CENTERED, 
SDL_WINDOWPOS_CENTERED);
+if (!window_x)
+window_x = SDL_WINDOWPOS_CENTERED;
+if (!window_y)
+window_y = SDL_WINDOWPOS_CENTERED;
+SDL_SetWindowPosition(window, window_x, window_y);
 if (is_full_screen)
 SDL_SetWindowFullscreen(window, SDL_WINDOW_FULLSCREEN_DESKTOP);
 SDL_ShowWindow(window);
@@ -3602,6 +3608,8 @@ static const OptionDef options[] = {
 { "framedrop", OPT_BOOL | OPT_EXPERT, {  }, "drop frames when 
cpu is too slow", "" },
 { "infbuf", OPT_BOOL | OPT_EXPERT, { _buffer }, "don't limit the 
input buffer size (useful with realtime streams)", "" },
 { "window_title", OPT_STRING | HAS_ARG, { _title }, "set window 
title", "window title" },
+{ "window_x", OPT_INT | HAS_ARG | OPT_EXPERT, { _x }, "set the x 
position of the window", "x pos" },
+{ "window_y", OPT_INT | HAS_ARG | OPT_EXPERT, { _y }, "set the y 
position of the window", "y pos" },
 #if CONFIG_AVFILTER
 { "vf", OPT_EXPERT | HAS_ARG, { .func_arg = opt_add_vfilter }, "set video 
filters", "filter_graph" },
 { "af", OPT_STRING | HAS_ARG, {  }, "set audio filters", 
"filter_graph" },
-- 
2.19.0


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

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: add reserve free space option

[Dave Rice]

> On Sep 12, 2018, at 11:56 AM, Sigríður Regína Sigurþórsdóttir 
>  wrote:
> 
> On Thu, Sep 6, 2018 at 3:31 PM James Almer  <mailto:jamr...@gmail.com>> wrote:
>> 
>> On 9/6/2018 4:18 PM, James Darnley wrote:
>>> On 2018-09-06 19:39, Sigríður Regína Sigurþórsdóttir wrote:
>>>> +if (s->metadata_header_padding) {
>>>> +if (s->metadata_header_padding == 1)
>>>> +s->metadata_header_padding++;
>>>> +put_ebml_void(pb, s->metadata_header_padding);
>>>> +}
>>> 
>>> Unfortunately I was forced to make the default -1 so you want to check
>>> that the value is greater than 0 rather than just true.
>>> 
>>> Furthermore I think you will still want to add to Changelog making a
>>> note that the matroska muxer will now listen to metadata_header_padding.
>> 
>> No, this kind of change doesn't justify a Changelog entry as mentioned
>> before.
>> 
>>> That may also want a micro version bump so that library users can check.
>> 
>> Micro version bump is ok.
> 
> 
> Thank you.
> 
> Here is an updated patch with a bump and a change to make sure the value is > 
> 0.
> 
> 
> 
> From 08e140fa0b23274a4db18ce0b201e45fe7c1ac97 Mon Sep 17 00:00:00 2001
> From: Sigga Regina mailto:siggareg...@gmail.com>>
> Date: Wed, 12 Sep 2018 11:47:47 -0400
> Subject: [PATCH] avformat/matroskaenc: add reserve free space option
> 
> ---
> libavformat/matroskaenc.c | 5 +
> libavformat/version.h | 2 +-
> 2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 09a62e1..3f5febf 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2005,6 +2005,11 @@ static int mkv_write_header(AVFormatContext *s)
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> +if (s->metadata_header_padding > 0) {
> +  if (s->metadata_header_padding == 1)
> +s->metadata_header_padding++;
> +  put_ebml_void(pb, s->metadata_header_padding);
> +}
> if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && mkv->reserve_cues_space) {
> mkv->cues_pos = avio_tell(pb);
> if (mkv->reserve_cues_space == 1)
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 4d21583..d7a1a35 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -33,7 +33,7 @@
> // Also please add any ticket numbers that you believe might be affected here
> #define LIBAVFORMAT_VERSION_MAJOR  58
> #define LIBAVFORMAT_VERSION_MINOR  18
> -#define LIBAVFORMAT_VERSION_MICRO 100
> +#define LIBAVFORMAT_VERSION_MICRO 101
> 
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>LIBAVFORMAT_VERSION_MINOR, \
> -- 
> 2.10.1 (Apple Git-78)
> <0001-avformat-matroskaenc-add-reserve-free-space-option 
> (1).patch>___

ping on this, as reserving such space in Matroska headers for later edits to 
the Tracks element would be helpful.
Dave Rice


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

Re: [FFmpeg-devel] Register for VDD 2018 conference

[Dave Rice]

>> On Aug 22, 2018, at 14:12, Michael Niedermayer  
>> wrote:
>> 
>> On Wed, Aug 22, 2018 at 03:14:53PM +0200, Jean-Baptiste Kempf wrote:
>> Hello fellow devs,
>> 
>> VideoLAN is happy to invite you to the usual conference of the end of the 
>> summer:
>> VDD2018 is happening in Paris, for the 10 years of the original conf.
>> 
>> As usual, this is a very technical conference focused on open source 
>> multimedia development.
>> We will talk about AV1, FFv1, FFv2, x264/x265, VLC, FFmpeg and other related 
>> technologies.
> 
> what is FFv2 ?

FFV2 is referenced in this patch http://akuvian.org/src/x264/ffv2.94.diff.

Also FFV2 is referenced as a derivative experimental work from Daala. 
https://twitter.com/atomnuker/status/924846477083578368?s=21

Also FFV2 is referenced by Reto Kromer as a forked alternative to the work on 
FFV1 version 4 by the IETF cellar working group. 
https://twitter.com/retokromer/status/884030201050648576?s=21

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

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On Jun 7, 2018, at 5:01 PM, Marton Balint  wrote:
> 
> On Thu, 7 Jun 2018, Dave Rice wrote:
> 
> [...]
> 
>> 
>> Before I only tested with vitc but now have a serial cable connected as well 
>> and found a source tape that has distinct values for LTC and VITC timecodes. 
>> The LTC values are from 1:00:00 to 2:00:00 and the VITC values are from 
>> 07:00:00 - 08:00:00.
>> 
>> With the deckcontrol utility at https://github.com/bavc/deckcontrol 
>> <https://github.com/bavc/deckcontrol>, I can use the command gettimecode to 
>> grab the LTC value:
>> 
>> deckcontrol gettimecode
>> Issued command 'gettimecode'
>> TC=07:37:56:21
>> Command sucessfully issued
>> Error sending command (No error)
>> 
>> With these patches, I can only grab the vitc values:
>> 
>> for i in rp188vitc rp188vitc2 rp188ltc rp188any vitc vitc2 serial ; do echo 
>> -n "${i}: " ; ./ffprobe -v quiet -timecode_format "$i" -f decklink 
>> -draw_bars 0 -audio_input embedded -video_input sdi -format_code ntsc 
>> -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -select_streams v 
>> -show_entries stream_tags=timecode -of default=nw=1:nk=1 ; echo ; done
>> rp188vitc: rp188vitc2: rp188ltc: rp188any: vitc: 01:41:44;06
>> vitc2: 01:41:44;21
>> serial:
>> 
>> Also it may be interesting in cases like this to support accepting multiple 
>> timecode inputs at once, such as "-timecode_format vitc+rp188ltc” though it 
>> would need to be contextualized more in metadata.
>> 
>> With a serial cable connected, I can access LTC via the deckcontrol utility 
>> but not with this patch.
> 
> Well, the way I understand it, deckcontrol is using a totally different 
> timecode source: the RS422 deck control interface. In contrast, the
> timecode capture in the patch is using the SDI (video) source.
> 
> If the deck does not put the LTC timecode into SDI line 10, then the driver 
> won't be able to capture it if you specify 'rp188ltc'. I am not sure however 
> why 'serial' does not work, but from a quick look at the SDK maybe that only 
> works if you use the deck control capture functions…

I see at 
https://forum.blackmagicdesign.com/viewtopic.php?f=12=71730=400097=bmdTimecodeSerial#p400097
 
<https://forum.blackmagicdesign.com/viewtopic.php?f=12=71730=400097=bmdTimecodeSerial#p400097>
 that capturing bmdTimecodeSerial is an issue there too, so this is likely an 
issue with the sdk rather than with your patch. Otherwise I’ve been testing 
this more and find it really useful. Hope to see this merged.
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On Jun 7, 2018, at 12:12 PM, Dave Rice  wrote:
> 
> 
>> On Jun 6, 2018, at 5:32 PM, Marton Balint  wrote:
>> 
>> On Wed, 6 Jun 2018, Dave Rice wrote:
>> 
>>>> On Jun 6, 2018, at 4:50 PM, Marton Balint  wrote:
>>>> On Mon, 4 Jun 2018, Dave Rice wrote:
>>>>>>> In my testing the timecode value set here has corrected been associated 
>>>>>>> with the first video frame (maintaining the timecode-to-first-frame 
>>>>>>> relationship as found on the source video stream). Although only having 
>>>>>>> first timecode value known is limiting, I think this is still quite 
>>>>>>> useful. This function also mirrors how BlackMagic Media Express and 
>>>>>>> Adobe Premiere handle capturing video+timecode where only the first 
>>>>>>> value is documented and all subsequent values are presumed.
>>>>>> Could you give me an example? (e.g. ffmpeg command line?)
>>>>> ./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input 
>>>>> embedded -video_input sdi -format_code ntsc -channels 8 -raw_format 
>>>>> yuv422p10 -i "UltraStudio 3D" -c:v v210 -c:a aac output.mov
>>>>> This worked for me to embed a QuickTime timecode track based upon the 
>>>>> timecode value of the first frame. If the input contained non-sequential 
>>>>> timecode values then the timecode track would not be accurate from that 
>>>>> point onward, but creating a timecode track based only upon the initial 
>>>>> value is what BlackMagic Media Express and Adobe Premiere are doing 
>>>>> anyhow.
>>>> Hmm, either the decklink drivers became better in hinding the first few 
>>>> NoSignal frames, or maybe that issue only affected to old models? (e.g. 
>>>> DeckLink SDI or DeckLink Duo 1). I did some test with a Mini Recorder, and 
>>>> even the first frame was useful, in this case the timecode was indeed 
>>>> correct.
>>>>>>>> I'd rather see a new AVPacketSideData type which will contain the 
>>>>>>>> timecode as a string, so you can set it frame-by-frame.
>>>>>>> Using side data for timecode would be preferable, but the possibility 
>>>>>>> that a patch for that may someday arrive shouldn’t completely block 
>>>>>>> this more limited patch.
>>>>>> I would like to make sure the code works reliably even for the limited 
>>>>>> use case and no race conditions are affectig the way it works.
>>>>> Feel welcome to suggest any testing. I’ll have access for testing again 
>>>>> tomorrow.
>>>> I reworked the patch a bit (see attached), and added per-frame timcode 
>>>> support into the PKT_STRINGS_METADATA packet side data, this way the 
>>>> drawtext filter can also be used to blend the timecode into the frames, 
>>>> which seems like a useful feature.
>>> 
>>> 
>>> That sounds helpful.
>>> 
>>> libavdevice/decklink_dec.cpp:734:21: error: unknown type name 'DECKLINK_STR'
>>>  DECKLINK_STR decklink_tc;
>> 
>> The patch I sent only replaces the second patch, the first one:
>> 
>> http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180526/185eb219/attachment.obj
>>  
>> <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180526/185eb219/attachment.obj>
> 
> Thanks for the update. I continued testing and found this very useful, 
> particularly with the side data.
> 
> Before I only tested with vitc but now have a serial cable connected as well 
> and found a source tape that has distinct values for LTC and VITC timecodes. 
> The LTC values are from 1:00:00 to 2:00:00 and the VITC values are from 
> 07:00:00 - 08:00:00.

Realized a mix up here, in the samples below VITC values are in 1:00:00 to 
2:00:00 and the LTC values are from 07:00:00 - 08:00:00

> With the deckcontrol utility at https://github.com/bavc/deckcontrol 
> <https://github.com/bavc/deckcontrol>, I can use the command gettimecode to 
> grab the LTC value:
> 
> deckcontrol gettimecode
> Issued command 'gettimecode'
> TC=07:37:56:21
> Command sucessfully issued
> Error sending command (No error)
> 
> With these patches, I can only grab the vitc values:
> 
> for i in rp188vitc rp188vitc2 rp188ltc rp188any vitc vitc2 serial ; do echo 
> -n "${i}: " ; ./ffprobe -v quiet -timecode_format "$i" -f decklink -draw_bars 
> 0 -audio_input embedded -video_in

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On Jun 6, 2018, at 5:32 PM, Marton Balint  wrote:
> 
> On Wed, 6 Jun 2018, Dave Rice wrote:
> 
>>> On Jun 6, 2018, at 4:50 PM, Marton Balint  wrote:
>>> On Mon, 4 Jun 2018, Dave Rice wrote:
>>>>>> In my testing the timecode value set here has corrected been associated 
>>>>>> with the first video frame (maintaining the timecode-to-first-frame 
>>>>>> relationship as found on the source video stream). Although only having 
>>>>>> first timecode value known is limiting, I think this is still quite 
>>>>>> useful. This function also mirrors how BlackMagic Media Express and 
>>>>>> Adobe Premiere handle capturing video+timecode where only the first 
>>>>>> value is documented and all subsequent values are presumed.
>>>>> Could you give me an example? (e.g. ffmpeg command line?)
>>>> ./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input 
>>>> embedded -video_input sdi -format_code ntsc -channels 8 -raw_format 
>>>> yuv422p10 -i "UltraStudio 3D" -c:v v210 -c:a aac output.mov
>>>> This worked for me to embed a QuickTime timecode track based upon the 
>>>> timecode value of the first frame. If the input contained non-sequential 
>>>> timecode values then the timecode track would not be accurate from that 
>>>> point onward, but creating a timecode track based only upon the initial 
>>>> value is what BlackMagic Media Express and Adobe Premiere are doing anyhow.
>>> Hmm, either the decklink drivers became better in hinding the first few 
>>> NoSignal frames, or maybe that issue only affected to old models? (e.g. 
>>> DeckLink SDI or DeckLink Duo 1). I did some test with a Mini Recorder, and 
>>> even the first frame was useful, in this case the timecode was indeed 
>>> correct.
>>>>>>> I'd rather see a new AVPacketSideData type which will contain the 
>>>>>>> timecode as a string, so you can set it frame-by-frame.
>>>>>> Using side data for timecode would be preferable, but the possibility 
>>>>>> that a patch for that may someday arrive shouldn’t completely block this 
>>>>>> more limited patch.
>>>>> I would like to make sure the code works reliably even for the limited 
>>>>> use case and no race conditions are affectig the way it works.
>>>> Feel welcome to suggest any testing. I’ll have access for testing again 
>>>> tomorrow.
>>> I reworked the patch a bit (see attached), and added per-frame timcode 
>>> support into the PKT_STRINGS_METADATA packet side data, this way the 
>>> drawtext filter can also be used to blend the timecode into the frames, 
>>> which seems like a useful feature.
>> 
>> 
>> That sounds helpful.
>> 
>> libavdevice/decklink_dec.cpp:734:21: error: unknown type name 'DECKLINK_STR'
>>   DECKLINK_STR decklink_tc;
> 
> The patch I sent only replaces the second patch, the first one:
> 
> http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180526/185eb219/attachment.obj
>  
> <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180526/185eb219/attachment.obj>

Thanks for the update. I continued testing and found this very useful, 
particularly with the side data.

Before I only tested with vitc but now have a serial cable connected as well 
and found a source tape that has distinct values for LTC and VITC timecodes. 
The LTC values are from 1:00:00 to 2:00:00 and the VITC values are from 
07:00:00 - 08:00:00.

With the deckcontrol utility at https://github.com/bavc/deckcontrol 
<https://github.com/bavc/deckcontrol>, I can use the command gettimecode to 
grab the LTC value:

deckcontrol gettimecode
Issued command 'gettimecode'
TC=07:37:56:21
Command sucessfully issued
Error sending command (No error)

With these patches, I can only grab the vitc values:

for i in rp188vitc rp188vitc2 rp188ltc rp188any vitc vitc2 serial ; do echo -n 
"${i}: " ; ./ffprobe -v quiet -timecode_format "$i" -f decklink -draw_bars 0 
-audio_input embedded -video_input sdi -format_code ntsc -channels 8 
-raw_format yuv422p10 -i "UltraStudio Express" -select_streams v -show_entries 
stream_tags=timecode -of default=nw=1:nk=1 ; echo ; done
rp188vitc: 
rp188vitc2: 
rp188ltc: 
rp188any: 
vitc: 01:41:44;06
vitc2: 01:41:44;21
serial: 

Also it may be interesting in cases like this to support accepting multiple 
timecode inputs at once, such as "-timecode_format vitc+rp188ltc” though it 
would need to be contextualized more in metadata.

With a serial cable connected, I can access LTC vi

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On Jun 6, 2018, at 4:50 PM, Marton Balint  wrote:
> 
> On Mon, 4 Jun 2018, Dave Rice wrote:
> 
>> 
>>>> In my testing the timecode value set here has corrected been associated 
>>>> with the first video frame (maintaining the timecode-to-first-frame 
>>>> relationship as found on the source video stream). Although only having 
>>>> first timecode value known is limiting, I think this is still quite 
>>>> useful. This function also mirrors how BlackMagic Media Express and Adobe 
>>>> Premiere handle capturing video+timecode where only the first value is 
>>>> documented and all subsequent values are presumed.
>>> Could you give me an example? (e.g. ffmpeg command line?)
>> 
>> ./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input 
>> embedded -video_input sdi -format_code ntsc -channels 8 -raw_format 
>> yuv422p10 -i "UltraStudio 3D" -c:v v210 -c:a aac output.mov
>> 
>> This worked for me to embed a QuickTime timecode track based upon the 
>> timecode value of the first frame. If the input contained non-sequential 
>> timecode values then the timecode track would not be accurate from that 
>> point onward, but creating a timecode track based only upon the initial 
>> value is what BlackMagic Media Express and Adobe Premiere are doing anyhow.
>> 
> 
> Hmm, either the decklink drivers became better in hinding the first few 
> NoSignal frames, or maybe that issue only affected to old models? (e.g. 
> DeckLink SDI or DeckLink Duo 1). I did some test with a Mini Recorder, and 
> even the first frame was useful, in this case the timecode was indeed correct.
> 
>>>>> I'd rather see a new AVPacketSideData type which will contain the 
>>>>> timecode as a string, so you can set it frame-by-frame.
>>>> Using side data for timecode would be preferable, but the possibility that 
>>>> a patch for that may someday arrive shouldn’t completely block this more 
>>>> limited patch.
>>> I would like to make sure the code works reliably even for the limited use 
>>> case and no race conditions are affectig the way it works.
>> 
>> Feel welcome to suggest any testing. I’ll have access for testing again 
>> tomorrow.
> 
> I reworked the patch a bit (see attached), and added per-frame timcode 
> support into the PKT_STRINGS_METADATA packet side data, this way the drawtext 
> filter can also be used to blend the timecode into the frames, which seems 
> like a useful feature.


That sounds helpful.

libavdevice/decklink_dec.cpp:734:21: error: unknown type name 'DECKLINK_STR'
DECKLINK_STR decklink_tc;

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

Re: [FFmpeg-devel] [PATCH 1/2] avdevice/decklink_dec: use a custom memory allocator

[Dave Rice]

> On Jun 5, 2018, at 1:17 PM, Marton Balint  wrote:
> 
> On Tue, 5 Jun 2018, Dave Rice wrote:
> 
>>> On Jun 4, 2018, at 4:21 PM, Marton Balint  wrote:
>>> 
>>> The default memory allocator is limited in the max number of frames 
>>> available,
>>> and therefore caused frame drops if the frames were not freed fast enough.
>> 
>> I’ve been testing this patchset today. Yesterday I was occasionally getting 
>> “Decklink input buffer overrun!” errors with this command:
>> 
>> /usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 
>> 1980 -f decklink -draw_bars 0 -audio_input embedded -video_input sdi 
>> -format_code ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" 
>> -metadata:s:v:0 encoder="FFV1 version 3" -color_primaries smpte170m 
>> -color_trc bt709 -colorspace smpte170m -color_range mpeg -metadata 
>> creation_time=now -f_strict unofficial -c:v ffv1 -level 3 -g 1 -slices 16 
>> -slicecrc 1 -c:a pcm_s24le -filter_complex 
>> "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; [0:a:0]pan=stereo| c0=c0 | 
>> c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | c1=c3[stereo2]" -map "[stereo1]" 
>> -map "[stereo2]" -f matroska output.mkv -an -f framemd5 output.framemd5
>> 
>> With the patchset applied, I haven’t had that buffer overrun error re-occur.
> 
> That is very strange, it should work the opposite way. Without the patch, the 
> decklink driver is dropping frames (silently), so you should never get a 
> Decklink input buffer overrun error message, but silent frame drops instead 
> if you don't release (transcode) the frames fast enough.
> 
> With the patch, you won't get silent frame drops, but you might fill the 
> internal queue and therefore get Decklink input buffer overruns. On the other 
> hand, if you get Decklink input buffer overruns, that typically means that 
> your computer is too slow to handle transcoding in real time…

Trying to detect unreported dropped frames is why I added the framemd5 output 
as a second output. After the command runs, I would use this command

grep -v "^#” output.framemd5 | awk -F',' '$2!=p+1{printf p+1"-"$2-1" "}{p=$2}'

to report the ranges of pts that weren’t incrementing the pts by 1 within the 
pts. I had presumed that getting a gap in the pts within the framemd5 was 
corresponding with the buffer overrun error in the terminal output. I’ve tested 
a few hours of recorded with your patch applied and haven’t gotten any pts 
discontinuity in the framemd5s yet.

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

Re: [FFmpeg-devel] [PATCH 1/2] avdevice/decklink_dec: use a custom memory allocator

[Dave Rice]

> On Jun 4, 2018, at 4:21 PM, Marton Balint  wrote:
> 
> The default memory allocator is limited in the max number of frames available,
> and therefore caused frame drops if the frames were not freed fast enough.

I’ve been testing this patchset today. Yesterday I was occasionally getting 
“Decklink input buffer overrun!” errors with this command:

/usr/local/opt/ffmpegdecklink/bin/ffmpeg-dl -v info -nostdin -nostats -t 1980 
-f decklink -draw_bars 0 -audio_input embedded -video_input sdi -format_code 
ntsc -channels 8 -raw_format yuv422p10 -i "UltraStudio Express" -metadata:s:v:0 
encoder="FFV1 version 3" -color_primaries smpte170m -color_trc bt709 
-colorspace smpte170m -color_range mpeg -metadata creation_time=now -f_strict 
unofficial -c:v ffv1 -level 3 -g 1 -slices 16 -slicecrc 1 -c:a pcm_s24le 
-filter_complex "[0:v:0]setfield=bff,setsar=40/27,setdar=4/3; 
[0:a:0]pan=stereo| c0=c0 | c1=c1[stereo1];[0:a:0]pan=stereo| c0=c2 | 
c1=c3[stereo2]" -map "[stereo1]" -map "[stereo2]" -f matroska output.mkv -an -f 
framemd5 output.framemd5

With the patchset applied, I haven’t had that buffer overrun error re-occur. 

> Signed-off-by: Marton Balint 
> ---
> libavdevice/decklink_dec.cpp | 50 
> 1 file changed, 50 insertions(+)
> 
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 510637676c..897fca1003 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -21,6 +21,9 @@
>  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
>  */
> 
> +#include 
> +using std::atomic;
> +
> /* Include internal.h first to avoid conflict between winsock.h (used by
>  * DeckLink headers) and winsock2.h (used by libavformat) in MSVC++ builds */
> extern "C" {
> @@ -98,6 +101,44 @@ static VANCLineNumber vanc_line_numbers[] = {
> {bmdModeUnknown, 0, -1, -1, -1}
> };
> 
> +class decklink_allocator : public IDeckLinkMemoryAllocator
> +{
> +public:
> +decklink_allocator(): _refs(1) { }
> +virtual ~decklink_allocator() { }
> +
> +// IDeckLinkMemoryAllocator methods
> +virtual HRESULT STDMETHODCALLTYPE AllocateBuffer(unsigned int 
> bufferSize, void* *allocatedBuffer)
> +{
> +void *buf = av_malloc(bufferSize + AV_INPUT_BUFFER_PADDING_SIZE);
> +if (!buf)
> +return E_OUTOFMEMORY;
> +*allocatedBuffer = buf;
> +return S_OK;
> +}
> +virtual HRESULT STDMETHODCALLTYPE ReleaseBuffer(void* buffer)
> +{
> +av_free(buffer);
> +return S_OK;
> +}
> +virtual HRESULT STDMETHODCALLTYPE Commit() { return S_OK; }
> +virtual HRESULT STDMETHODCALLTYPE Decommit() { return S_OK; }
> +
> +// IUnknown methods
> +virtual HRESULT STDMETHODCALLTYPE QueryInterface(REFIID iid, LPVOID 
> *ppv) { return E_NOINTERFACE; }
> +virtual ULONG   STDMETHODCALLTYPE AddRef(void) { return ++_refs; }
> +virtual ULONG   STDMETHODCALLTYPE Release(void)
> +{
> +int ret = --_refs;
> +if (!ret)
> +delete this;
> +return ret;
> +}
> +
> +private:
> +std::atomic  _refs;
> +};
> +
> extern "C" {
> static void decklink_object_free(void *opaque, uint8_t *data)
> {
> @@ -924,6 +965,7 @@ av_cold int ff_decklink_read_header(AVFormatContext 
> *avctx)
> {
> struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> struct decklink_ctx *ctx;
> +class decklink_allocator *allocator;
> AVStream *st;
> HRESULT result;
> char fname[1024];
> @@ -1017,6 +1059,14 @@ av_cold int ff_decklink_read_header(AVFormatContext 
> *avctx)
> ctx->input_callback = new decklink_input_callback(avctx);
> ctx->dli->SetCallback(ctx->input_callback);
> 
> +allocator = new decklink_allocator();
> +ret = (ctx->dli->SetVideoInputFrameMemoryAllocator(allocator) == S_OK ? 
> 0 : AVERROR_EXTERNAL);
> +allocator->Release();
> +if (ret < 0) {
> +av_log(avctx, AV_LOG_ERROR, "Cannot set custom memory allocator\n");
> +goto error;
> +}
> +
> if (mode_num == 0 && !cctx->format_code) {
> if (decklink_autodetect(cctx) < 0) {
> av_log(avctx, AV_LOG_ERROR, "Cannot Autodetect input stream or No 
> signal\n");
> -- 
> 2.16.3
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On Jun 4, 2018, at 12:24 PM, Marton Balint  wrote:
> 
> On Fri, 1 Jun 2018, Dave Rice wrote:
> 
>>> On May 31, 2018, at 5:29 PM, Marton Balint  wrote:
>>> On Thu, 31 May 2018, Jonathan Morley wrote:
>>>> Thank you for the clarification, Dave. It might be that the Blackmagic 
>>>> approach to collecting timecode doesn’t work for that one source because 
>>>> it is in the horizontal space (HANC) instead of the vertical (VANC). I am 
>>>> not sure. Sadly I don’t think my solution is all encompassing, but it does 
>>>> seem to help in enough cases I would like to get it integrated with master.
>>>> I am still a bit thrown about the initial “Unable to set timecode” error, 
>>>> but believe it to be initialization related. I will wait to hear back from 
>>>> Marton on my overall approach and see what I can do to clean that up.
>>> av_dict_set returns < 0 on error, so the condition seems wrong.
>>>> As for the other error message my plan is to demote that to a debug.
>>> That is a good idea.
>> 
>> +1
>> 
>>> On the other hand, I believe the usefulness of this is in its current form 
>>> is still very limited, because typically the first few frames are NoSignal 
>>> frames, how the end user supposed to know which frame is the one with the 
>>> first valid timecode?
>> 
>> In my testing the timecode value set here has corrected been associated with 
>> the first video frame (maintaining the timecode-to-first-frame relationship 
>> as found on the source video stream). Although only having first timecode 
>> value known is limiting, I think this is still quite useful. This function 
>> also mirrors how BlackMagic Media Express and Adobe Premiere handle 
>> capturing video+timecode where only the first value is documented and all 
>> subsequent values are presumed.
> 
> Could you give me an example? (e.g. ffmpeg command line?)

./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input embedded 
-video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i 
"UltraStudio 3D" -c:v v210 -c:a aac output.mov

This worked for me to embed a QuickTime timecode track based upon the timecode 
value of the first frame. If the input contained non-sequential timecode values 
then the timecode track would not be accurate from that point onward, but 
creating a timecode track based only upon the initial value is what BlackMagic 
Media Express and Adobe Premiere are doing anyhow.

>>> I'd rather see a new AVPacketSideData type which will contain the timecode 
>>> as a string, so you can set it frame-by-frame.
>> 
>> Using side data for timecode would be preferable, but the possibility that a 
>> patch for that may someday arrive shouldn’t completely block this more 
>> limited patch.
> 
> I would like to make sure the code works reliably even for the limited use 
> case and no race conditions are affectig the way it works.

Feel welcome to suggest any testing. I’ll have access for testing again 
tomorrow.
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On May 31, 2018, at 5:29 PM, Marton Balint  wrote:
> 
> On Thu, 31 May 2018, Jonathan Morley wrote:
> 
>> Thank you for the clarification, Dave. It might be that the Blackmagic 
>> approach to collecting timecode doesn’t work for that one source because it 
>> is in the horizontal space (HANC) instead of the vertical (VANC). I am not 
>> sure. Sadly I don’t think my solution is all encompassing, but it does seem 
>> to help in enough cases I would like to get it integrated with master.
>> 
>> I am still a bit thrown about the initial “Unable to set timecode” error, 
>> but believe it to be initialization related. I will wait to hear back from 
>> Marton on my overall approach and see what I can do to clean that up.
> 
> av_dict_set returns < 0 on error, so the condition seems wrong.
> 
>> 
>> As for the other error message my plan is to demote that to a debug.
> 
> That is a good idea.

+1

> On the other hand, I believe the usefulness of this is in its current form is 
> still very limited, because typically the first few frames are NoSignal 
> frames, how the end user supposed to know which frame is the one with the 
> first valid timecode?

In my testing the timecode value set here has corrected been associated with 
the first video frame (maintaining the timecode-to-first-frame relationship as 
found on the source video stream). Although only having first timecode value 
known is limiting, I think this is still quite useful. This function also 
mirrors how BlackMagic Media Express and Adobe Premiere handle capturing 
video+timecode where only the first value is documented and all subsequent 
values are presumed.

> I'd rather see a new AVPacketSideData type which will contain the timecode as 
> a string, so you can set it frame-by-frame.

Using side data for timecode would be preferable, but the possibility that a 
patch for that may someday arrive shouldn’t completely block this more limited 
patch.

[…]

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

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

Hi Jonathan,

> On May 31, 2018, at 3:56 PM, Jonathan Morley  wrote:
> 
> Well that is helpful information if not a bit disappointing. Perhaps if I use 
> the SDK calls to get the individual timecode components _and_ check the drop 
> frame flag I can reassemble what the GetString() method on the 
> IDeckLinkTimecode class is supposed to provide.

I tried with another NDF tape and it worked appropriately. So I do have a 
capture with NDF and another capture with DF timecode as intended. I am still 
uncertain what’s wrong with the first NDF that I reported before but I’m 
supposing it’s an issue with that tape rather than your work.

Here’s the output of the capture of the new NDF tape, note that I’m still 
getting the "Unable to set timecode” even though the timecode is set.

./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input embedded 
-video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i 
"UltraStudio 3D" -c:v v210 -c:a aac maybendf.mov
ffmpeg version N-91200-g1616b1be5a Copyright (c) 2000-2018 the FFmpeg developers
  built with Apple LLVM version 9.0.0 (clang-900.0.38)
  configuration: --enable-nonfree --enable-decklink 
--extra-cflags=-I/usr/local/include --extra-ldflags=-L/usr/local/include
  libavutil  56. 18.102 / 56. 18.102
  libavcodec 58. 19.104 / 58. 19.104
  libavformat58. 17.100 / 58. 17.100
  libavdevice58.  4.100 / 58.  4.100
  libavfilter 7. 24.100 /  7. 24.100
  libswscale  5.  2.100 /  5.  2.100
  libswresample   3.  2.100 /  3.  2.100
[decklink @ 0x7fdd75800a00] Found Decklink mode 720 x 486 with rate 29.97(i)
[decklink @ 0x7fdd75800a00] Unable to set timecode
Guessed Channel Layout for Input Stream #0.0 : 7.1
Input #0, decklink, from 'UltraStudio 3D':
  Duration: N/A, start: 0.00, bitrate: 229869 kb/s
Stream #0:0: Audio: pcm_s16le, 48000 Hz, 7.1, s16, 6144 kb/s
Stream #0:1: Video: v210 (V210 / 0x30313256), yuv422p10le(bottom first), 
720x486, 223725 kb/s, 29.97 fps, 29.97 tbr, 1000k tbn, 1000k tbc
Metadata:
  timecode: 03:48:55:26
Stream mapping:
  Stream #0:1 -> #0:0 (v210 (native) -> v210 (native))
  Stream #0:0 -> #0:1 (pcm_s16le (native) -> aac (native))
Press [q] to stop, [?] for help
Output #0, mov, to 'maybendf.mov':
  Metadata:
encoder : Lavf58.17.100
Stream #0:0: Video: v210 (v210 / 0x30313276), yuv422p10le(bottom coded 
first (swapped)), 720x486, q=2-31, 223725 kb/s, 0.03 fps, 30k tbn, 29.97 tbc
Metadata:
  timecode: 03:48:55:26
  encoder : Lavc58.19.104 v210
Stream #0:1: Audio: aac (LC) (mp4a / 0x6134706D), 48000 Hz, 7.1, fltp, 469 
kb/s
Metadata:
  encoder : Lavc58.19.104 aac
frame=   48 fps= 34 q=-0.0 Lsize=   43745kB time=00:00:01.56 
bitrate=228505.8kbits/s speed=1.12x
video:43740kB audio:2kB subtitle:0kB other streams:0kB global headers:0kB 
muxing overhead: 0.007146%
[aac @ 0x7fdd77801e00] Qavg: 65536.000

> I will add that to the next patch, but it might be a minute before I get to 
> it. Thank you again for taking a look.
> 
> I guess we got really lucky in our use case.
> 
> Thanks,
> Jon
> 
>> On May 31, 2018, at 12:39 PM, Dave Rice  wrote:
>> 
>> Hi Jonathan,
>> 
>>> On May 31, 2018, at 11:41 AM, Jonathan Morley >> <mailto:jmor...@pixsystem.com>> wrote:
>>> 
>>> Thank you very much, Dave. I am really curious about the df vs ndf since 
>>> the Blackmagic SDK call I am making doesn’t have any arguments for 
>>> specifying that kind of distinction. It simply returns what it finds in the 
>>> SDI stream.
>> 
>> I know have a tape known to be NDF and a tape known to be DF. The messages I 
>> sent before were from DF tapes. When I tried a NDF tape, I get the "Unable 
>> to find timecode” warning repeatedly and no timecode on the output file.
>> 
>> ./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input 
>> embedded -video_input sdi -format_code ntsc -channels 8 -raw_format 
>> yuv422p10 -i "UltraStudio 3D" -c:v v210 -c:a aac devlin5.mov
>> ffmpeg version N-91200-g1616b1be5a Copyright (c) 2000-2018 the FFmpeg 
>> developers
>> built with Apple LLVM version 9.0.0 (clang-900.0.38)
>> configuration: --enable-nonfree --enable-decklink 
>> --extra-cflags=-I/usr/local/include --extra-ldflags=-L/usr/local/include
>> libavutil  56. 18.102 / 56. 18.102
>> libavcodec 58. 19.104 / 58. 19.104
>> libavformat58. 17.100 / 58. 17.100
>> libavdevice58.  4.100 / 58.  4.100
>> libavfilter 7. 24.100 /  7. 24.100
>> libswscale  5.  2.100 /  5.  2.100
>> libswresample   3.  2.100 /  3.  2.100
>> [decklink @ 0x7f9f6680] Found Decklink mode 720 x 486 with rate 29.97(i)
>> [decklink @ 0x7f9f668000

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

Hi Jonathan,

> On May 31, 2018, at 11:41 AM, Jonathan Morley  wrote:
> 
> Thank you very much, Dave. I am really curious about the df vs ndf since the 
> Blackmagic SDK call I am making doesn’t have any arguments for specifying 
> that kind of distinction. It simply returns what it finds in the SDI stream.

I know have a tape known to be NDF and a tape known to be DF. The messages I 
sent before were from DF tapes. When I tried a NDF tape, I get the "Unable to 
find timecode” warning repeatedly and no timecode on the output file.

./ffmpeg -timecode_format vitc2 -f decklink -draw_bars 0 -audio_input embedded 
-video_input sdi -format_code ntsc -channels 8 -raw_format yuv422p10 -i 
"UltraStudio 3D" -c:v v210 -c:a aac devlin5.mov
ffmpeg version N-91200-g1616b1be5a Copyright (c) 2000-2018 the FFmpeg developers
  built with Apple LLVM version 9.0.0 (clang-900.0.38)
  configuration: --enable-nonfree --enable-decklink 
--extra-cflags=-I/usr/local/include --extra-ldflags=-L/usr/local/include
  libavutil  56. 18.102 / 56. 18.102
  libavcodec 58. 19.104 / 58. 19.104
  libavformat58. 17.100 / 58. 17.100
  libavdevice58.  4.100 / 58.  4.100
  libavfilter 7. 24.100 /  7. 24.100
  libswscale  5.  2.100 /  5.  2.100
  libswresample   3.  2.100 /  3.  2.100
[decklink @ 0x7f9f6680] Found Decklink mode 720 x 486 with rate 29.97(i)
[decklink @ 0x7f9f6680] Unable to find timecode.
Last message repeated 5 times
Guessed Channel Layout for Input Stream #0.0 : 7.1
Input #0, decklink, from 'UltraStudio 3D':
  Duration: N/A, start: 0.00, bitrate: 229869 kb/s
Stream #0:0: Audio: pcm_s16le, 48000 Hz, 7.1, s16, 6144 kb/s
Stream #0:1: Video: v210 (V210 / 0x30313256), yuv422p10le(bottom first), 
720x486, 223725 kb/s, 29.97 fps, 29.97 tbr, 1000k tbn, 1000k tbc
Stream mapping:
  Stream #0:1 -> #0:0 (v210 (native) -> v210 (native))
  Stream #0:0 -> #0:1 (pcm_s16le (native) -> aac (native))
Press [q] to stop, [?] for help
Output #0, mov, to 'devlin5.mov':
  Metadata:
encoder : Lavf58.17.100
Stream #0:0: Video: v210 (v210 / 0x30313276), yuv422p10le(bottom coded 
first (swapped)), 720x486, q=2-31, 223725 kb/s, 29.97 fps, 30k tbn, 29.97 tbc
Metadata:
  encoder : Lavc58.19.104 v210
Stream #0:1: Audio: aac (LC) (mp4a / 0x6134706D), 48000 Hz, 7.1, fltp, 469 
kb/s
Metadata:
  encoder : Lavc58.19.104 aac
[decklink @ 0x7f9f6680] Unable to find timecode.
Last message repeated 15 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:00.70 
bitrate=200517.8kbits/s speed= 1.4x
Last message repeated 14 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:01.20 
bitrate=211246.0kbits/s speed= 1.2x
Last message repeated 14 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:01.70 
bitrate=214431.3kbits/s speed=1.13x
Last message repeated 14 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:02.20 
bitrate=217121.0kbits/s speed= 1.1x
Last message repeated 14 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:02.70 
bitrate=221142.3kbits/s speed=1.07x
Last message repeated 14 times
[decklink @ 0x7f9f6680] Unable to find timecode.0:03.20 
bitrate=221288.2kbits/s speed=1.06x

So when inputing DF this appears to work while Media Express gets it wrong by 
writing the timecode with a NDF flag.
However when inputing NDF no timecode is communicated.

I ran this to check if somehow I was called the wrong timecode 

for i in rp188vitc rp188vitc2 rp188ltc rp188any vitc vitc2 serial ; do echo -n 
"${i}: " ; ./ffprobe -v quiet -timecode_format "$i" -f decklink -draw_bars 0 
-audio_input embedded -video_input sdi -format_code ntsc -channels 8 
-raw_format yuv422p10 -i "UltraStudio 3D" -select_streams v -show_entries 
stream_tags=timecode -of default=nw=1:nk=1 ; echo ; done

rp188vitc: 
rp188vitc2: 
rp188ltc: 
rp188any: 
vitc: 
vitc2: 
serial: 

I may try to find another NDF tape to make sure this isn’t a fluke.

> And when I skimmed the movenc timecode handling it doesn’t seem to make any 
> assumptions or changes either.

I’ve replicated it with Matroska which simply moves the timecode into metadata 
as a string.

> Please keep me posted. Meanwhile I will look into what could be causing all 
> the error chatter.

Thanks!

> Thanks,
> Jon
> 
>> On May 31, 2018, at 7:59 AM, Dave Rice  wrote:
>> 
>> 
>>> On May 31, 2018, at 5:49 AM, Jonathan Morley  wrote:
>>> 
>>> Please take a look at my latest patches.
>>> 
>>> NOTE: I no longer have the hardware to test this work!
>> 
>> I tested these patches with an Ultrastudio 3D.
>> 
>> I find that in some cases it provides the “Unable to set timecode” warning 
>> although it does provide the timecode value. Such as:
>> 
>> ./ffmpeg -timec

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

e=218719.7kbits/s speed=1.11x
Last message repeated 14 times
[decklink @ 0x7fe313803000] Unable to find timecode.0:02.16 
bitrate=219494.2kbits/s speed=1.08x
Last message repeated 14 times
[decklink @ 0x7fe313803000] Unable to find timecode.0:02.66 
bitrate=220763.9kbits/s speed=1.07x
Last message repeated 14 times
[decklink @ 0x7fe313803000] Unable to find timecode.0:03.16 
bitrate=220971.1kbits/s speed=1.06x
Last message repeated 14 times
[decklink @ 0x7fe313803000] Unable to find timecode.0:03.67 
bitrate=221693.2kbits/s speed=1.05x
Last message repeated 14 times
[decklink @ 0x7fe313803000] Unable to find timecode.0:04.17 
bitrate=223247.5kbits/s speed=1.04x  

Until I hit the play button on my source.

I tested the same videotape sources with Media Express and ffmpeg with this 
patch and the initial timecode values are aligned to the same frames 
accurately, but with 4 random videotapes I’ve tried, all the Media Express 
captures are Non-Drop Frame and all the ffmpeg captures are Drop Frame, so one 
has to be wrong. I’ll try to find a source that can provide a known df and ndf 
signal to determine which is correct.

Thanks for the update,
Dave Rice

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

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On May 25, 2018, at 17:15, Jonathan Morley  wrote:
> 
> Thank you for trying it out, Dave.
> 
> 1) Can you please tell me more about how you tested?

J30 digibeta deck playing a Betacam SP with SDI to an Ultrastudio 3D with 
ffmpeg with your patch and configured with decklink. 

> 2) Did you run a capture command with ffmpeg after patching?

Yes. And I should have shared that command in my email. From memory it was 
something like: ffmpeg -f decklink $input-format-options-for-an-sd-ntsc-input 
-i “Ultrastudio 3D” -c:v v210 -c:a aac output.mov

> 3) What was the target output you we using?

QuickTime since the -metadata timecode=# tag works well there.

I can test more on Tuesday. Thanks for this work!

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

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

Re: [FFmpeg-devel] [PATCH] Limited timecode support for lavd/decklink

[Dave Rice]

> On May 25, 2018, at 1:08 PM, Jonathan Morley  wrote:
> 
>> On May 25, 2018, at 8:32 AM, Marton Balint > > wrote:
>> 
>> On Fri, 25 May 2018, Jonathan Morley wrote:
>> 
>>> I apologize in advance for the formatting of this message. My mail
>>> configuration impeded my efforts to use ‘—send-email’.
>> 
>> Your mail is corrupted by new lines, attach the .patch file if you cannot 
>> resolve this.
> 
> Will do! Sorry about that. The mail server’s auth setup kept me from using 
> ‘—send-email’ directly. I will send future patches as attachments.

Here is a copy of my adjustments for the patch so that I could apply it for 
testing. https://gist.github.com/dericed/af69d60a99ad235b17722a64ea9413ba 

>>> This commit did pass 'make fate’ though the use of these changes requires
>>> decklink hardware so I did not add any new fate tests. By default the new
>>> optional behavior is skipped for old behavior.
>>> 
>>> ---
>>> libavdevice/decklink_common.cpp | 30 -
>>> libavdevice/decklink_common.h   | 42 ++
>>> +++
>> 
>> Why is this huge chunk of function movement needed? If there is a good 
>> reason to do that, then split it into a separate patch, if there is not, 
>> then just get rid of it.
> 
> The move here was so that I could make use of the cross platform safe 
> DECKLINK string methods. I was only able to test with linux and build on mac, 
> but I wanted to do my best to make a contribution that _could_ work on all 
> platforms. I can break this into two patches if you prefer. 
> 
>>> libavdevice/decklink_common_c.h |  1 +
>>> libavdevice/decklink_dec.cpp| 19 +++
>>> libavdevice/decklink_dec_c.c|  9 +
>>> 5 files changed, 71 insertions(+), 30 deletions(-)
>>> 
>>> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.
>>> cpp
>>> index d8cced7c74..aab9d85b94 100644
>>> --- a/libavdevice/decklink_common.cpp
>>> +++ b/libavdevice/decklink_common.cpp
>>> @@ -77,36 +77,6 @@ static IDeckLinkIterator
>>> *decklink_create_iterator(AVFormatContext
>>> *avctx)
>>>  return iter;
>>> }
>>> 
>>> -#ifdef _WIN32
>>> -static char *dup_wchar_to_utf8(wchar_t *w)
>>> -{
>>> -char *s = NULL;
>>> -int l = WideCharToMultiByte(CP_UTF8, 0, w, -1, 0, 0, 0, 0);
>>> -s = (char *) av_malloc(l);
>>> -if (s)
>>> -WideCharToMultiByte(CP_UTF8, 0, w, -1, s, l, 0, 0);
>>> -return s;
>>> -}
>>> -#define DECKLINK_STROLECHAR *
>>> -#define DECKLINK_STRDUP dup_wchar_to_utf8
>>> -#define DECKLINK_FREE(s) SysFreeString(s)
>>> -#elif defined(__APPLE__)
>>> -static char *dup_cfstring_to_utf8(CFStringRef w)
>>> -{
>>> -char s[256];
>>> -CFStringGetCString(w, s, 255, kCFStringEncodingUTF8);
>>> -return av_strdup(s);
>>> -}
>>> -#define DECKLINK_STRconst __CFString *
>>> -#define DECKLINK_STRDUP dup_cfstring_to_utf8
>>> -#define DECKLINK_FREE(s) CFRelease(s)
>>> -#else
>>> -#define DECKLINK_STRconst char *
>>> -#define DECKLINK_STRDUP av_strdup
>>> -/* free() is needed for a string returned by the DeckLink SDL. */
>>> -#define DECKLINK_FREE(s) free((void *) s)
>>> -#endif
>>> -
>>> HRESULT ff_decklink_get_display_name(IDeckLink *This, const char
>>> **displayName)
>>> {
>>>  DECKLINK_STR tmpDisplayName;
>>> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
>>> index 57ee7d1d68..8c5f8e9f06 100644
>>> --- a/libavdevice/decklink_common.h
>>> +++ b/libavdevice/decklink_common.h
>>> @@ -34,6 +34,36 @@
>>> #define DECKLINK_BOOL bool
>>> #endif
>>> 
>>> +#ifdef _WIN32
>>> +static char *dup_wchar_to_utf8(wchar_t *w)
>>> +{
>>> +char *s = NULL;
>>> +int l = WideCharToMultiByte(CP_UTF8, 0, w, -1, 0, 0, 0, 0);
>>> +s = (char *) av_malloc(l);
>>> +if (s)
>>> +WideCharToMultiByte(CP_UTF8, 0, w, -1, s, l, 0, 0);
>>> +return s;
>>> +}
>>> +#define DECKLINK_STROLECHAR *
>>> +#define DECKLINK_STRDUP dup_wchar_to_utf8
>>> +#define DECKLINK_FREE(s) SysFreeString(s)
>>> +#elif defined(__APPLE__)
>>> +static char *dup_cfstring_to_utf8(CFStringRef w)
>>> +{
>>> +char s[256];
>>> +CFStringGetCString(w, s, 255, kCFStringEncodingUTF8);
>>> +return av_strdup(s);
>>> +}
>>> +#define DECKLINK_STRconst __CFString *
>>> +#define DECKLINK_STRDUP dup_cfstring_to_utf8
>>> +#define DECKLINK_FREE(s) CFRelease(s)
>>> +#else
>>> +#define DECKLINK_STRconst char *
>>> +#define DECKLINK_STRDUP av_strdup
>>> +/* free() is needed for a string returned by the DeckLink SDL. */
>>> +#define DECKLINK_FREE(s) free((void *) s)
>>> +#endif
>>> +
>>> class decklink_output_callback;
>>> class decklink_input_callback;
>>> 
>>> @@ -64,6 +94,7 @@ struct decklink_ctx {
>>>  BMDDisplayMode bmd_mode;
>>>  BMDVideoConnection video_input;
>>>  BMDAudioConnection audio_input;
>>> +BMDTimecodeFormat tc_format;
>>>  int bmd_width;
>>>  

Re: [FFmpeg-devel] [RFC][ALT PATCHES] Code of Conduct Enforcement

[Dave Rice]

> On May 17, 2018, at 10:22 AM, Clément Bœsch <u...@pkh.me> wrote:
> 
> On Mon, May 14, 2018 at 05:50:25PM +0100, Derek Buitenhuis wrote:
> [...]
>>1. Implement a formal CoC enforcement system. This has been mostly 
>> copypasted
>>   from VideoLAN's, and is meant as more of a blueprint. This will no 
>> doubt
>>   be controversial.
> 
> So as mentioned already in the thread, the main issue is having a
> police/justice entity. I would say it needs to be separate from the
> development team (to maintain a power separation). Since such profile
> doesn't seem to be exactly common in the open source world, maybe we could
> externalize it. Does such a service exist with reasonable prices? Could we
> use our funds for this? I understand this may sound far-fetched, but who
> knows.

CoC enforcement as a paid service sounds alarming. Though it might make sense 
to consider people separate from the development team for the role. There are 
likely many who would like to contribute to the FFmpeg project but not as a 
developer who could consider such a role.

> If such solution is not viable, we could fallback on the voting committee
> to elect/design a subgroup of itself (an odd number like 3 persons maybe?)
> to hold this moderation task for a period of 3 or 6 months, maybe 1 year.
> Then these members are automatically maintainer of the CoC for this period
> of time, and decide what to do with it.
> 
> Just random thoughts, no hard opinion on it to be honest.

I like this suggestion for a small committee to be tasked and trusted with such 
actions. I consider that it might be easier to find rough consensus in scenario 
a than in scenario b.

a) the larger ffmpeg community finds consensus to appoint a CoC committee and 
as needed the CoC committee finds consensus (as a small group) on how to 
respond to concerns from the community and to implement the CoC.

b) the larger ffmpeg community finds consensus on how to implement the CoC 
directly each time there’s a concern from the community.

Dave Rice

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

Re: [FFmpeg-devel] [RFC] Exporting virtual timelines as stream side data

[Dave Rice]

> On Mar 27, 2018, at 5:16 PM, wm4 <nfx...@googlemail.com> wrote:
> 
> On Tue, 27 Mar 2018 16:45:23 -0400
> Dave Rice <d...@dericed.com <mailto:d...@dericed.com>> wrote:
> 
>>> On Mar 27, 2018, at 4:33 PM, wm4 <nfx...@googlemail.com> wrote:
>>> 
>>> On Tue, 27 Mar 2018 16:11:11 -0400
>>> Dave Rice <d...@dericed.com> wrote:
>>> 
>>>>> On Mar 27, 2018, at 4:01 PM, Derek Buitenhuis 
>>>>> <derek.buitenh...@gmail.com> wrote:
>>>>> 
>>>>> On 3/27/2018 8:52 PM, Rostislav Pehlivanov wrote:
>>>>>> I think we should drop the internal crap if the tools and the API support
>>>>>> it. Would also solve a lot of issues like ffmpeg.c not trimming the start
>>>>>> frame (so people complain all the time about longer files).
>>>>> 
>>>>> I personally agree, but I thought I'd be diplomatic about it, since it 
>>>>> would
>>>>> technically be losing a 'feature', since it would no longer Just Work(ish)
>>>>> and require user applications to apply timelines themselves - and I 
>>>>> figured
>>>>> some would argue that point.
>>>> 
>>>> +1 I’m willing to contribute what information or samples would be needed 
>>>> to help with Matroska support with virtual timelines. IMO, this would be a 
>>>> valuable feature to have in ffmpeg.
>>>> Dave Rice  
>>> 
>>> Some explanations how this interacts with editions would be good.  
>> 
>> I put an example with two editions at 
>> https://archive.org/download/chapters_test/chapters_test.mkv which mimics a 
>> digitized video 
> 
> Also this file lacks a chapter end time in the second edition. How is
> that valid?

You’re right. I moved this discussion to cellar, but when the Edition is 
Ordered then ChapterTimeEnd is required.
Dave Rice

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

Re: [FFmpeg-devel] [RFC] Exporting virtual timelines as stream side data

[Dave Rice]

> On Mar 27, 2018, at 4:33 PM, wm4 <nfx...@googlemail.com> wrote:
> 
> On Tue, 27 Mar 2018 16:11:11 -0400
> Dave Rice <d...@dericed.com> wrote:
> 
>>> On Mar 27, 2018, at 4:01 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
>>> wrote:
>>> 
>>> On 3/27/2018 8:52 PM, Rostislav Pehlivanov wrote:  
>>>> I think we should drop the internal crap if the tools and the API support
>>>> it. Would also solve a lot of issues like ffmpeg.c not trimming the start
>>>> frame (so people complain all the time about longer files).  
>>> 
>>> I personally agree, but I thought I'd be diplomatic about it, since it would
>>> technically be losing a 'feature', since it would no longer Just Work(ish)
>>> and require user applications to apply timelines themselves - and I figured
>>> some would argue that point.  
>> 
>> +1 I’m willing to contribute what information or samples would be needed to 
>> help with Matroska support with virtual timelines. IMO, this would be a 
>> valuable feature to have in ffmpeg.
>> Dave Rice
> 
> Some explanations how this interacts with editions would be good.

I put an example with two editions at 
https://archive.org/download/chapters_test/chapters_test.mkv which mimics a 
digitized video tape. One edition depicts the stored video encoding in its 
entire presentation and the other default edition plays from the video encoding 
selectively. When the file is played in VLC, the default edition is used to 
show a shortened presentation, but the other editions may be selected within 
the Program menu option. In the proposal there should be an option to mimic the 
current behavior and ignore the editions (equivalent of -ignore_editlist in 
mov), to use the first default edition, or to select a specific edition.

> Or documenting ordered chapters and segment linking in the mkv "spec"
> at all.

This document, http://mod16.org/hurfdurf/?p=8 <http://mod16.org/hurfdurf/?p=8>, 
 is unofficially a good resource on that.

More officially the Cellar working group has this section on Linked Segments: 
https://tools.ietf.org/html/draft-lhomme-cellar-matroska-04#section-23 
<https://tools.ietf.org/html/draft-lhomme-cellar-matroska-04#section-23>, and 
this one on Ordered Editions: 
https://tools.ietf.org/html/draft-lhomme-cellar-matroska-04#section-10.1.2.3 
<https://tools.ietf.org/html/draft-lhomme-cellar-matroska-04#section-10.1.2.3>. 
This documentation is a work in progress so comments/reviews/suggestions are 
welcome (issues can be filed at 
https://github.com/Matroska-Org/matroska-specification 
<https://github.com/Matroska-Org/matroska-specification> or via 
https://www.ietf.org/mailman/listinfo/cellar 
<https://www.ietf.org/mailman/listinfo/cellar>).

Dave Rice

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

Re: [FFmpeg-devel] [RFC] Exporting virtual timelines as stream side data

[Dave Rice]

> On Mar 27, 2018, at 4:01 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 3/27/2018 8:52 PM, Rostislav Pehlivanov wrote:
>> I think we should drop the internal crap if the tools and the API support
>> it. Would also solve a lot of issues like ffmpeg.c not trimming the start
>> frame (so people complain all the time about longer files).
> 
> I personally agree, but I thought I'd be diplomatic about it, since it would
> technically be losing a 'feature', since it would no longer Just Work(ish)
> and require user applications to apply timelines themselves - and I figured
> some would argue that point.

+1 I’m willing to contribute what information or samples would be needed to 
help with Matroska support with virtual timelines. IMO, this would be a 
valuable feature to have in ffmpeg.
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [Cellar] [PATCH] avcodec/ffv1: Support for RGBA64 and GBRAP16

[Dave Rice]

> On Feb 5, 2018, at 4:20 PM, Jerome Martinez <jer...@mediaarea.net> wrote:
> 
> On 03/02/2018 14:48, Michael Niedermayer wrote:
>> 
>> I hope this will not reduce interrest in working on a improved
>> 9-16bit mode in v4.
> 
> I don't like to put politics in technical stuff, but here this is politics, 
> and I think that blocking an easy improvement of FFV1 v3 encoding/decoding in 
> FFmpeg (actually just using the current FFV1 v3, and also v1 actually, 
> specification without having artificial limitations about bit depths for a 
> specific color space, i.e. 16-bit support was already there for YUV before I 
> work on FFV1) just for forcing people to do a completely different work (e.g. 
> working on FFV1 v4) is not fair and IMO is not in the idea behind open source.
> IMO if interest for 9-16bit (side note: I don't see why 8-bit should not be 
> in the range, some ideas I have for v4 are useful also for 8-bit) mode in v4 
> is reduced, that would only mean that v3 is already so great and/or just that 
> people have no better idea right now, it is not bad, and both works (using v3 
> at the maximum of its possibilities and working on v4) are separate works. 
> FYI criticism I saw about FFV1 is not compression ratio but speed (playing a 
> 4K stream is just impossible on a "normal" machine, you need a very expensive 
> CPU for that) so my plan is to focus on speed improvement in priority. It 
> could be v4 stuff (incompatible changes), or v3 (encoder/decoder 
> optimization), depending of what I find.

IMHO, improved higher bit depths in version 4 is very interesting. We already 
have a few noted exceptions where version 4 is intended to fix and optimize a 
few things of version 3 (signed 16 bit handling, RGB plane order for 9-15 
bits), so a change in optimization of high bit depth handling seems already 
appropriate for consideration.

>> [...]
>> 
>>> Last but not least, since almost two years now it's impossible
>>> to work on the development of FFV1 v4. It's always the wrong
>>> time and/or the wrong place every time I am doing something for
>>> this cause. Why? This is extremely frustrating.
>> I want to understand this too. IMO v4 development should be more
>> important than or at least equally to the v3 draft polishing and neither
>> should block the other.
> 
> Also politics, but I don't understand who is blocking v4 from your point of 
> view. "impossible to work on v4"? Where? As far as I see 1 person (me) said 
> his priority is having FFV1 v3 becoming a standard (so IETF related work) and 
> widely used (so any bit depth for any supported color space in v3 because it 
> is an easy demonstration that FFV1 is versatile without having to wait for v4 
> R, especially because v3 bitstream design is already pretty good and 
> versatile) because this is what I need, and my personal case is not blocking 
> anyone else for sending patches for either FFV1 specifications or FFmpeg code 
> about v4. Eager to see patch proposals for v4 (and v3 if it does not break 
> support of files in the wild), whoever feeling blocked with his patches 
> should send patch proposals to either FFmpeg (code) or CELLAR mailing list 
> (bitstream format).
> 
> That said, I plan to add more pix_fmt support for FFV1 v3 (which is already a 
> good format!) support in FFmpeg and some optimization ideas beside my work 
> for IETF spec, with the hope we could speak about technical issues (e.g. more 
> optimization hints or info about wrong code or warning about that it breaks 
> the bitstream specs as currently written), leaving aside politics.

In the cellar charter and timeline, the effort to produce a informational 
specification for FFV1 video codec versions 0, 1 and 3 precedes the effort to 
produce a specification for FFV1 video codec version 4. Initially I anticipated 
that the version 4 specification would be a fork from the completed version 
0,1,3 draft; however, I think the current approach (one doc that ‘makes’ both 
outputs) works well to allow version 4 work to proceed without blocking any 
remaining version 0,1,3 work. Still I suggest that we resolve 
https://github.com/FFmpeg/FFV1/pull/87 and 
https://github.com/FFmpeg/FFV1/pull/71 soon as IMHO the version 0,1,3 
informational specification is very close to a last call and submission to the 
IESG (chairs are welcome to offer other opinions ;) ). So while I encourage 
resolution to these pull requests, it seems we have a good system to manage 
concurrent work on both FFV1 goals of the charter.

Kind Regards,
Dave Rice


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

Re: [FFmpeg-devel] [PATCH] ffmpeg.c: use sigterm_handler with sigpipe

[Dave Rice]

Thread bump.

> On Jan 11, 2018, at 5:51 PM, Nicolas George <geo...@nsup.org> wrote:
> 
> Moritz Barsnick (2018-01-11):
>> This patch doesn't change the handling of SIGTERM
> 
> You should have read SIGPIPE, obviously.
> 
>> Is SIGPIPE an interactive signal?
> 
> Of course not.
> 
>>  Anything on the other side of output
>> file(name) "-" or "pipe:N" may terminate for some reason.
> 
> Yes, that is exactly what SIGPIPE is for.
> 
>> This patch does NOT try to ignore anything. ffmpeg won't keep running
>> due to ignoring of SIGPIPE, it will terminate more cleanly due to
>> handling it. The former is not desired. (But yes, shall handing to
>> enforce ignoring it would allow that.)
> 
> It will terminate less cleanly than if you do the right thing with
> SIGPIPE.

This patch has been working for me and ffmpeg terminates cleanly with SIGPIPE 
with a valid output (moov atom written with mov or cues/seekhead written with 
mkv). Not sure if I understand well the disadvantage of this patch.
Dave Rice

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

[FFmpeg-devel] [PATCH] ffmpeg.c: use sigterm_handler with sigpipe

[Dave Rice]

From 0faa2954010feb8428542fc33aa81e898a280c88 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 11 Jan 2018 15:52:36 -0500
Subject: [PATCH] ffmpeg.c: use sigterm_handler with sigpipe

Based on a suggestion by Moritz Barsnick at 
http://ffmpeg.org/pipermail/ffmpeg-user/2018-January/038549.html 
<http://ffmpeg.org/pipermail/ffmpeg-user/2018-January/038549.html>

---
 fftools/ffmpeg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 0c16e75ab0..dfcc865dcf 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -406,6 +406,7 @@ void term_init(void)
 
 signal(SIGINT , sigterm_handler); /* Interrupt (ANSI).*/
 signal(SIGTERM, sigterm_handler); /* Termination (ANSI).  */
+signal(SIGPIPE, sigterm_handler); /* Termination (pipe closed).  */
 #ifdef SIGXCPU
 signal(SIGXCPU, sigterm_handler);
 #endif
-- 
2.15.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/avf_showwaves: add draw grid support

[Dave Rice]

> On Nov 21, 2017, at 7:36 AM, Paul B Mahol <one...@gmail.com> wrote:
> 
> Signed-off-by: Paul B Mahol <one...@gmail.com>
> ---
> doc/filters.texi|  6 ++
> libavfilter/avf_showwaves.c | 28 
> 2 files changed, 34 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 62f633c6f8..9c98f1684b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -19178,6 +19178,12 @@ Cubic root.
> @end table
> 
> Default is linear.
> +
> +@item grid
> +Draw grid, default is disabled.
> +
> +@item grid_color
> +Set grid color.
> @end table
> 
> @subsection Examples
> diff --git a/libavfilter/avf_showwaves.c b/libavfilter/avf_showwaves.c
> index 0866967984..74d4886cd4 100644
> --- a/libavfilter/avf_showwaves.c
> +++ b/libavfilter/avf_showwaves.c
> @@ -69,6 +69,8 @@ typedef struct ShowWavesContext {
> int mode;   ///< ShowWavesMode
> int scale;  ///< ShowWavesScale
> int split_channels;
> +int grid;
> +uint8_t grid_rgba[4];
> uint8_t *fg;
> 
> int (*get_h)(int16_t sample, int height);
> @@ -104,6 +106,8 @@ static const AVOption showwaves_options[] = {
> { "log", "logarithmic",0, AV_OPT_TYPE_CONST, {.i64=SCALE_LOG}, 
> .flags=FLAGS, .unit="scale"},
> { "sqrt", "square root",   0, AV_OPT_TYPE_CONST, {.i64=SCALE_SQRT}, 
> .flags=FLAGS, .unit="scale"},
> { "cbrt", "cubic root",0, AV_OPT_TYPE_CONST, {.i64=SCALE_CBRT}, 
> .flags=FLAGS, .unit="scale"},
> +{ "grid", "draw grid", OFFSET(grid), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, 
> FLAGS },
> +{ "grid_color", "set grid color", OFFSET(grid_rgba), AV_OPT_TYPE_COLOR, 
> {.str="0x00"}, 0, 0, FLAGS },
> { NULL }
> };
> 
> @@ -562,6 +566,30 @@ static int alloc_out_frame(ShowWavesContext *showwaves, 
> const int16_t *p,
>   outlink->time_base);
> for (j = 0; j < outlink->h; j++)
> memset(out->data[0] + j*out->linesize[0], 0, outlink->w * 
> showwaves->pixstep);
> +
> +if (showwaves->grid) {
> +const int pixstep = showwaves->pixstep;
> +int ystep = showwaves->split_channels ? outlink->h / 
> inlink->channels / 4 : outlink->h / 4;
> +int channels = showwaves->split_channels ? inlink->channels : 1;
> +int x, s, c, yskip = 0;
> +
> +for (c = 0; c < channels; c++) {
> +for (j = 0; j < 4; j++) {
> +for (x = 0; x < outlink->w; x+=3) {
> +for (s = 0; s < pixstep; s++) {
> +out->data[0][(yskip + j * ystep) * 
> out->linesize[0] + x * pixstep + s] = showwaves->grid_rgba[s];
> +}
> +}
> +}
> +for (x = 0; x < outlink->w; x+=3) {
> +for (s = 0; s < pixstep; s++) {
> +out->data[0][(yskip + j * ystep - 1) * 
> out->linesize[0] + x * pixstep + s] = showwaves->grid_rgba[s];
> +}
> +}
> +
> +yskip += j * ystep;
> +}
> +}
> }
> return 0;
> }
> -- 
> 2.11.0

Seems interesting but do the gridlines convey any meaning? Perhaps a flags 
value too similar to waveform that should label the lines in dB or as a float. 
Also perhaps worth adjustment the placement of the gridlines depending on a 
scale (log vs lin).
Dave Rice

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 11:22 AM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 11/20/2017 3:19 PM, Dave Rice wrote:
>> TN2162 requires a colr atom for uncompressed yuv (including v210, v308, 
>> v408, etc) in mov, so I'd prefer to write it in this case. Note that the 
>> colr atom provides an option for unspecified for each of the color values, 
>> so there's a method to write a colr atom which basically says ¯\_(ツ)_/¯.
> 
> [...]
> 
>> I disagree. I'd prefer to follow the specification and write a colr atom (in 
>> the case of uncompressed yuv in mov) that say the colr is unspecified rather 
>> than to write no colr atom at all and create an invalid file. See 
>> https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9.
>> 
>> I do agree that guesswork should be avoided or provided under an option that 
>> users could opt into. I suppose my preference order from support to oppose 
>> for writing uncompressed yuv in mov would be:
>> 
>> 1: write a colr atom in all cases (if unknown, use unspecified values in 
>> colr atom)
>> 2: write a colr atom if color data the known (no colr atom if unknown)
>> 3: write a colr atom in all cases (if unknown, just make stuff up, #yolo)
> 
> My opinion falls somewhere in between, to something like this (in 
> pseudo-code):
> 
>if (colors_known) {
>write_colr(vals);
>} else if (uncompressed_yuv) {
>if (guesswork_user_option) {
>write_colr(guessed_hacky_crap);
>} else {
>write_colr(unspecified);
>}
>} else if (guesswork_user_option) {
>write_colr(guessed_hacky_crap);
>} else {
>// Don't write a colr atom because it adds no value, to write 
> unspecified in it
>// and no spec requires it for compressed streams.
>}

What do you propose as the default for guessed_hacky_crap? Also are there 
supporters for the need of a guessed_hacky_crap optio? Is there precedence in 
ffmpeg to enable/disable guesswork via a user option?
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 9:55 AM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 11/20/2017 2:50 PM, Dave Rice wrote:
>> I am hesitant to see write_colr option removed since the guesswork used here 
>> https://github.com/FFmpeg/FFmpeg/blob/8f4702a93f87f9f76563e80f1ae2141a40029d9d/libavformat/movenc.c#L1747-L1775
>>  would then be applied to all movs of standard HD and SD size and cause many 
>> ffmpeg outputs to appear differently after the change.
> 
> 100% agree with this. We *should not* write guessed color information based
> off of resolution by default.
> 
> Further, if no color information is present, no colr atom should be written
> at all, unless it is forced by the user (aka the write_colr option right
> now).

TN2162 requires a colr atom for uncompressed yuv (including v210, v308, v408, 
etc) in mov, so I'd prefer to write it in this case. Note that the colr atom 
provides an option for unspecified for each of the color values, so there's a 
method to write a colr atom which basically says ¯\_(ツ)_/¯.

>> Could the guesswork here be removed? Or could write_colr be enabled with the 
>> option removed while the guesswork itself has its own option?
> 
> If the quesswork is removed or put under an option, I still maintain no
> colr atom should be written if no color information is known. Doing
> so is just asking for trouble, in my opinion.

I disagree. I'd prefer to follow the specification and write a colr atom (in 
the case of uncompressed yuv in mov) that say the colr is unspecified rather 
than to write no colr atom at all and create an invalid file. See 
https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG9.

I do agree that guesswork should be avoided or provided under an option that 
users could opt into. I suppose my preference order from support to oppose for 
writing uncompressed yuv in mov would be:

1: write a colr atom in all cases (if unknown, use unspecified values in colr 
atom)
2: write a colr atom if color data the known (no colr atom if unknown)
3: write a colr atom in all cases (if unknown, just make stuff up, #yolo)

Dave Rice

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

Re: [FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

> On Nov 20, 2017, at 9:01 AM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
> 
> 2017-11-20 2:24 GMT+01:00 James Almer <jamr...@gmail.com 
> <mailto:jamr...@gmail.com>>:
>> On 11/18/2017 11:19 PM, Dave Rice wrote:
>>> From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
>>> From: Dave Rice <d...@dericed.com>
>>> Date: Sat, 18 Nov 2017 20:31:27 -0500
>>> Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv 
>>> in
>>> mov
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>> 
>>> As required by Apple’s TN2162.
>>> ---
>>> libavformat/movenc.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index aaa1dedfd7..86960b19c1 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
>>> MOVMuxContext *mov, MOVTrack *tr
>>> else
>>> av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. 
>>> Format is not MOV.\n");
>>> }
>>> -if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
>>> +if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
>>> if (track->mode == MODE_MOV || track->mode == MODE_MP4)
>>> mov_write_colr_tag(pb, track);
>>> else
>>> 
>> 
>> The write_colr option says "Write colr atom (Experimental, may be
>> renamed or changed, do not use from scripts)". Does that still apply? Is
>> the feature/spec still experimental?
>> 
>> If not, then the option and flag could be removed as well as part of
>> this patch.
> 
> I believe it should be removed in a follow-up patch.

I am hesitant to see write_colr option removed since the guesswork used here 
https://github.com/FFmpeg/FFmpeg/blob/8f4702a93f87f9f76563e80f1ae2141a40029d9d/libavformat/movenc.c#L1747-L1775
 would then be applied to all movs of standard HD and SD size and cause many 
ffmpeg outputs to appear differently after the change.

Could the guesswork here be removed? Or could write_colr be enabled with the 
option removed while the guesswork itself has its own option?
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter/vf_tile: add queue option

[Dave Rice]

> On Nov 19, 2017, at 2:19 PM, Nicolas George <geo...@nsup.org> wrote:
> 
> Paul B Mahol (2017-11-19):
>> So what you propose?
> 
> I do not know. It is your patch, and I am not even sure I understood
> your explanation correctly.

IMHO, ‘queue' is a good name for what it does. Perhaps an additional example 
would help demonstrate the option better. For example `ffplay -f lavfi -i 
testsrc=r=1 -vf tile=1x8:overlap=7:queue=1` will display 8 adjoining frames of 
the testsrc. Without `queue=1` in this example, the output will be stalled for 
8 seconds before displaying, whereas with `queue=1` there will be an output 
frame for each input frame (the first frame of the output simply showing the 
first frame of the input plus seven empty spaces).

Suggestion:

@item
Produce a filmstrip animation from frame @code{n-7} to @code{n}:
@example
ffmpeg -i file.avi -vf 'tile=1x8:overlap=7:queue=1' filmstrip.mkv
@end example

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

[FFmpeg-devel] [PATCH 4/4] fate: add v210 mov test

[Dave Rice]

From fe9e3aa13ecf3b4cb81f279696bcc21602662e7a Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Sat, 18 Nov 2017 20:32:33 -0500
Subject: [PATCH 4/4] fate: add v210 mov test

---
 tests/fate/vcodec.mak | 4 
 tests/ref/vsynth/vsynth1-mov-v210 | 4 
 2 files changed, 8 insertions(+)
 create mode 100644 tests/ref/vsynth/vsynth1-mov-v210

diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak
index bbcf25d72a..0206312a53 100644
--- a/tests/fate/vcodec.mak
+++ b/tests/fate/vcodec.mak
@@ -360,6 +360,10 @@ fate-vsynth%-mov-bpp16:  CODEC   = rawvideo
 fate-vsynth%-mov-bpp16:  ENCOPTS = -pix_fmt rgb565le
 fate-vsynth%-mov-bpp16:  FMT  = mov
 
+FATE_VCODEC-$(call ENCDEC, V210, MOV) += mov-v210
+fate-vsynth%-mov-v210:  CODEC = v210
+fate-vsynth%-mov-v210:  FMT   = mov
+
 FATE_VCODEC-$(call ENCDEC, ROQ, ROQ)+= roqvideo
 fate-vsynth%-roqvideo:   CODEC   = roqvideo
 fate-vsynth%-roqvideo:   ENCOPTS = -frames 5
diff --git a/tests/ref/vsynth/vsynth1-mov-v210 
b/tests/ref/vsynth/vsynth1-mov-v210
new file mode 100644
index 00..035f8df6ff
--- /dev/null
+++ b/tests/ref/vsynth/vsynth1-mov-v210
@@ -0,0 +1,4 @@
+a96ffa7a9ccb8242cb462dd698b3e222 *tests/data/fate/vsynth1-mov-v210.mov
+14746427 tests/data/fate/vsynth1-mov-v210.mov
+2ba7f4ca302f3c4147860b9dfb12b6e4 *tests/data/fate/vsynth1-mov-v210.out.rawvideo
+stddev:1.84 PSNR: 42.81 MAXDIFF:   29 bytes:  7603200/  7603200
-- 
2.15.0

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

[FFmpeg-devel] [PATCH 1/4] avformat/movenc: correct ImageDescription for uncompressed ycbcr

[Dave Rice]

This patch set updates movenc to write uncompressed ycbcr in mov following 
requirements in Apple’s TN2162, 
https://developer.apple.com/library/content/technotes/tn2162/_index.html. 
Thanks to Carl and Michael for comments on an earlier version of this patchset.


From 26d9ca470f104d8448000b13c2cc97b8fc5c15ba Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:53:32 -0500
Subject: [PATCH 1/4] avformat/movenc: correct ImageDescription for
 uncompressed ycbcr

Per
https://developer.apple.com/library/content/technotes/tn2162/_index.html
.
---
 libavformat/movenc.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index cc3fc19d9b..ce51c4b3d2 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1832,6 +1832,13 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 char compressor_name[32] = { 0 };
 int avid = 0;
 
+int uncompressed_ycbcr = ((track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_UYVY422)
+   || (track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_YUYV422)
+   ||  track->par->codec_id == AV_CODEC_ID_V308
+   ||  track->par->codec_id == AV_CODEC_ID_V408
+   ||  track->par->codec_id == AV_CODEC_ID_V410
+   ||  track->par->codec_id == AV_CODEC_ID_V210);
+
 avio_wb32(pb, 0); /* size */
 if (mov->encryption_scheme != MOV_ENC_NONE) {
 ffio_wfourcc(pb, "encv");
@@ -1842,11 +1849,15 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 avio_wb16(pb, 0); /* Reserved */
 avio_wb16(pb, 1); /* Data-reference index */
 
-avio_wb16(pb, 0); /* Codec stream version */
+if (uncompressed_ycbcr) {
+avio_wb16(pb, 2); /* Codec stream version */
+} else {
+avio_wb16(pb, 0); /* Codec stream version */
+}
 avio_wb16(pb, 0); /* Codec stream revision (=0) */
 if (track->mode == MODE_MOV) {
 ffio_wfourcc(pb, "FFMP"); /* Vendor */
-if (track->par->codec_id == AV_CODEC_ID_RAWVIDEO) {
+if (track->par->codec_id == AV_CODEC_ID_RAWVIDEO || 
uncompressed_ycbcr) {
 avio_wb32(pb, 0); /* Temporal Quality */
 avio_wb32(pb, 0x400); /* Spatial Quality = lossless*/
 } else {
@@ -1870,7 +1881,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 avio_w8(pb, strlen(compressor_name));
 avio_write(pb, compressor_name, 31);
 
-if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
+if (track->mode == MODE_MOV &&
+   (track->par->codec_id == AV_CODEC_ID_V410 || track->par->codec_id == 
AV_CODEC_ID_V210))
+avio_wb16(pb, 0x18);
+else if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
 avio_wb16(pb, track->par->bits_per_coded_sample |
   (track->par->format == AV_PIX_FMT_GRAY8 ? 0x20 : 0));
 else
-- 
2.15.0

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

[FFmpeg-devel] [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in mov

[Dave Rice]

From 41da5e48f8788b85dd7a382030bb2866c506cc03 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Sat, 18 Nov 2017 20:31:27 -0500
Subject: [PATCH 3/4] avformat/movenc: force colr atom for uncompressed yuv in
 mov
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As required by Apple’s TN2162.
---
 libavformat/movenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index aaa1dedfd7..86960b19c1 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1978,7 +1978,7 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 else
 av_log(mov->fc, AV_LOG_WARNING, "Not writing 'gama' atom. Format 
is not MOV.\n");
 }
-if (mov->flags & FF_MOV_FLAG_WRITE_COLR) {
+if (mov->flags & FF_MOV_FLAG_WRITE_COLR || uncompressed_ycbcr) {
 if (track->mode == MODE_MOV || track->mode == MODE_MP4)
 mov_write_colr_tag(pb, track);
 else
-- 
2.15.0

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

[FFmpeg-devel] [PATCH 2/4] avformat/movenc: write clap atom for uncompressed yuv in mov

[Dave Rice]

From e9079167c199791e652fe9cd3550333b819a8a9a Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:29:06 -0500
Subject: [PATCH 2/4] avformat/movenc: write clap atom for uncompressed yuv in
 mov

fixes 6145
---
 libavformat/movenc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index ce51c4b3d2..aaa1dedfd7 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1686,6 +1686,21 @@ static int mov_write_sv3d_tag(AVFormatContext *s, 
AVIOContext *pb, AVSphericalMa
 return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+avio_wb32(pb, 40);
+ffio_wfourcc(pb, "clap");
+avio_wb32(pb, track->par->width); /* apertureWidth_N */
+avio_wb32(pb, 1); /* apertureWidth_D (= 1) */
+avio_wb32(pb, track->height); /* apertureHeight_N */
+avio_wb32(pb, 1); /* apertureHeight_D (= 1) */
+avio_wb32(pb, 0); /* horizOff_N (= 0) */
+avio_wb32(pb, 1); /* horizOff_D (= 1) */
+avio_wb32(pb, 0); /* vertOff_N (= 0) */
+avio_wb32(pb, 1); /* vertOff_D (= 1) */
+return 40;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1984,6 +1999,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 mov_write_pasp_tag(pb, track);
 }
 
+if (uncompressed_ycbcr){
+mov_write_clap_tag(pb, track);
+}
+
 if (mov->encryption_scheme != MOV_ENC_NONE) {
 ff_mov_cenc_write_sinf_tag(track, pb, mov->encryption_kid);
 }
-- 
2.15.0

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

Re: [FFmpeg-devel] [PATCH] avformat/movenc: correct ImageDescription depth for v210 v410

[Dave Rice]

> On Nov 16, 2017, at 6:08 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
> 
> 2017-11-16 17:54 GMT+01:00 Dave Rice <d...@dericed.com>:
> 
>> +if (track->mode == MODE_MOV && track->par->codec_id == AV_CODEC_ID_V410)
>> +avio_wb16(pb, 0x18);
>> +else if (track->mode == MODE_MOV && track->par->codec_id == 
>> AV_CODEC_ID_V210)
>> +avio_wb16(pb, 0x18);
> 
> It appears you can merge the two cases.

The patch is updated with merged cases below.

> Or maybe patch bits_per_coded_sample in the encoder…


With Apple’s TN2162 there doesn’t appear to be a reliable relationship between 
the bits_per_coded_sample and what the ImageDescription depth value should be 
for the uncompressed yuv formats. TN2162 simply lists what the depth value 
should be and this patch corrects the few instances, where ffmpeg’s behavior 
doesn’t correlate to what TN2162 defines.


From cfa5b2cd959154f2896a9557d9ca2ed2d2d3834e Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:53:32 -0500
Subject: [PATCH 2/2] avformat/movenc: correct ImageDescription depth for v210
 v410

Per
https://developer.apple.com/library/content/technotes/tn2162/_index.html
.
---
 libavformat/movenc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 98fcc7a44b..d9d3c2bf1e 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1896,7 +1896,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 avio_w8(pb, strlen(compressor_name));
 avio_write(pb, compressor_name, 31);
 
-if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
+if (track->mode == MODE_MOV &&
+   (track->par->codec_id == AV_CODEC_ID_V410 || track->par->codec_id == 
AV_CODEC_ID_V210))
+avio_wb16(pb, 0x18);
+else if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
 avio_wb16(pb, track->par->bits_per_coded_sample |
   (track->par->format == AV_PIX_FMT_GRAY8 ? 0x20 : 0));
 else
-- 
2.15.0

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

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Dave Rice]

> On Nov 16, 2017, at 11:30 AM, Dave Rice <d...@dericed.com> wrote:
> 
>> On Jul 9, 2017, at 7:26 PM, Dave Rice <d...@dericed.com> wrote:
>> 
>>> On Jul 7, 2017, at 7:06 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
>>> wrote:
>>> 
>>> On 7/7/2017 10:13 PM, James Almer wrote:
>>>> Isn't this necessary only for files with raw video? As is, this box
>>>> would be written on any mov file with a video stream.
>>> 
>>> This was addressed a previous email:
>>> 
>>>  http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html
>>> 
>>> I guess the spec is up for interpretation.
>> 
>> The quicktime spec says "This is a mandatory extension for all uncompressed 
>> Y´CbCr data formats”. It doesn’t clarify if the clap atom is recommended or 
>> not in mov files that are not “uncompressed Y´CbCr”, though it would make 
>> sense if the video container needs to store cropping data. I think 
>> constraining the change for only  “uncompressed Y´CbCr” would be more 
>> cautious though. I’ll revise my patch to include the condition and resubmit.
>> 
>> If the patch only impacts “uncompressed Y´CbCr” would any fate updates be 
>> needed?
>> Dave Rice
> 
> Here’s an update to only write the clap atom for the specific uncompressed 
> encodings listed in TN2162.
> 
> From 37457c1ee135f39452b91b047af4adf1ec43464b Mon Sep 17 00:00:00 2001
> From: Dave Rice <d...@dericed.com>
> Date: Thu, 16 Nov 2017 11:29:06 -0500
> Subject: [PATCH] avformat/movenc: write clap atom for uncompressed yuv in mov

Sorry, this patch should supersede the prior email's patch. I realized that 
Apple requires new uncompressed ycbcr files to use version 2 in the Image 
Description, so I reused the uncompressed_ycbcr variable to add that in as well.

From 3ea99e7d22f67b8a556152acbcbc8bf2eeec8a39 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:29:06 -0500
Subject: [PATCH 1/2] avformat/movenc: write clap atom for uncompressed yuv in
 mov

fixes 6145
---
 libavformat/movenc.c | 32 +++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index cc3fc19d9b..98fcc7a44b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1686,6 +1686,21 @@ static int mov_write_sv3d_tag(AVFormatContext *s, 
AVIOContext *pb, AVSphericalMa
 return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+avio_wb32(pb, 40);
+ffio_wfourcc(pb, "clap");
+avio_wb32(pb, track->par->width); /* apertureWidth_N */
+avio_wb32(pb, 1); /* apertureWidth_D (= 1) */
+avio_wb32(pb, track->height); /* apertureHeight_N */
+avio_wb32(pb, 1); /* apertureHeight_D (= 1) */
+avio_wb32(pb, 0); /* horizOff_N (= 0) */
+avio_wb32(pb, 1); /* horizOff_D (= 1) */
+avio_wb32(pb, 0); /* vertOff_N (= 0) */
+avio_wb32(pb, 1); /* vertOff_D (= 1) */
+return 40;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1832,6 +1847,13 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 char compressor_name[32] = { 0 };
 int avid = 0;
 
+int uncompressed_ycbcr = ((track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_UYVY422)
+   || (track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_YUYV422)
+   ||  track->par->codec_id == AV_CODEC_ID_V308
+   ||  track->par->codec_id == AV_CODEC_ID_V408
+   ||  track->par->codec_id == AV_CODEC_ID_V410
+   ||  track->par->codec_id == AV_CODEC_ID_V210);
+
 avio_wb32(pb, 0); /* size */
 if (mov->encryption_scheme != MOV_ENC_NONE) {
 ffio_wfourcc(pb, "encv");
@@ -1842,7 +1864,11 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 avio_wb16(pb, 0); /* Reserved */
 avio_wb16(pb, 1); /* Data-reference index */
 
-avio_wb16(pb, 0); /* Codec stream version */
+if (uncompressed_ycbcr) {
+avio_wb16(pb, 2); /* Codec stream version */
+} else {
+avio_wb16(pb, 0); /* Codec stream version */
+}
 avio_wb16(pb, 0); /* Codec stream revision (=0) */
 if (track->mode == MODE_MOV) {
 ffio_wfourcc(pb, "FFMP"); /* Vendor */
@@ -1969,6 +1995,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 if (track->par->sample_aspect_ratio.den && 
track->par->sample_aspect_ratio.num) {
 mov_w

[FFmpeg-devel] [PATCH] avformat/movenc: correct ImageDescription depth for v210 v410

[Dave Rice]

This corrects a few values in the Image Description for v210 and v410 in mov. 
Apple defines what the depth value for these uncompressed formats should be in 
https://developer.apple.com/library/content/technotes/tn2162/_index.html 
<https://developer.apple.com/library/content/technotes/tn2162/_index.html>.


From 47def189b270ac93245e002580463b254daf8484 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:53:32 -0500
Subject: [PATCH] avformat/movenc: correct ImageDescription depth for v210 v410

Per
https://developer.apple.com/library/content/technotes/tn2162/_index.html
.
---
 libavformat/movenc.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 18232e8ba3..f7b08e2885 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1885,7 +1885,11 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 avio_w8(pb, strlen(compressor_name));
 avio_write(pb, compressor_name, 31);
 
-if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
+if (track->mode == MODE_MOV && track->par->codec_id == AV_CODEC_ID_V410)
+avio_wb16(pb, 0x18);
+else if (track->mode == MODE_MOV && track->par->codec_id == 
AV_CODEC_ID_V210)
+avio_wb16(pb, 0x18);
+else if (track->mode == MODE_MOV && track->par->bits_per_coded_sample)
 avio_wb16(pb, track->par->bits_per_coded_sample |
   (track->par->format == AV_PIX_FMT_GRAY8 ? 0x20 : 0));
 else
-- 
2.15.0

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

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Dave Rice]

> On Jul 9, 2017, at 7:26 PM, Dave Rice <d...@dericed.com> wrote:
> 
> 
>> On Jul 7, 2017, at 7:06 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
>> wrote:
>> 
>> On 7/7/2017 10:13 PM, James Almer wrote:
>>> Isn't this necessary only for files with raw video? As is, this box
>>> would be written on any mov file with a video stream.
>> 
>> This was addressed a previous email:
>> 
>>   http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html
>> 
>> I guess the spec is up for interpretation.
> 
> The quicktime spec says "This is a mandatory extension for all uncompressed 
> Y´CbCr data formats”. It doesn’t clarify if the clap atom is recommended or 
> not in mov files that are not “uncompressed Y´CbCr”, though it would make 
> sense if the video container needs to store cropping data. I think 
> constraining the change for only  “uncompressed Y´CbCr” would be more 
> cautious though. I’ll revise my patch to include the condition and resubmit.
> 
> If the patch only impacts “uncompressed Y´CbCr” would any fate updates be 
> needed?
> Dave Rice

Here’s an update to only write the clap atom for the specific uncompressed 
encodings listed in TN2162.

From 37457c1ee135f39452b91b047af4adf1ec43464b Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 16 Nov 2017 11:29:06 -0500
Subject: [PATCH] avformat/movenc: write clap atom for uncompressed yuv in mov

fixes 6145
---
 libavformat/movenc.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index cc3fc19d9b..18232e8ba3 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1686,6 +1686,21 @@ static int mov_write_sv3d_tag(AVFormatContext *s, 
AVIOContext *pb, AVSphericalMa
 return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+avio_wb32(pb, 40);
+ffio_wfourcc(pb, "clap");
+avio_wb32(pb, track->par->width); /* apertureWidth_N */
+avio_wb32(pb, 1); /* apertureWidth_D (= 1) */
+avio_wb32(pb, track->height); /* apertureHeight_N */
+avio_wb32(pb, 1); /* apertureHeight_D (= 1) */
+avio_wb32(pb, 0); /* horizOff_N (= 0) */
+avio_wb32(pb, 1); /* horizOff_D (= 1) */
+avio_wb32(pb, 0); /* vertOff_N (= 0) */
+avio_wb32(pb, 1); /* vertOff_D (= 1) */
+return 40;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1970,6 +1985,16 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 mov_write_pasp_tag(pb, track);
 }
 
+int uncompressed_ycbcr = ((track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_UYVY422)
+   || (track->par->codec_id == AV_CODEC_ID_RAWVIDEO && 
track->par->format == AV_PIX_FMT_YUYV422)
+   ||  track->par->codec_id == AV_CODEC_ID_V308
+   ||  track->par->codec_id == AV_CODEC_ID_V408
+   ||  track->par->codec_id == AV_CODEC_ID_V410
+   ||  track->par->codec_id == AV_CODEC_ID_V210);  

+if (uncompressed_ycbcr){
+mov_write_clap_tag(pb, track);
+}
+
 if (mov->encryption_scheme != MOV_ENC_NONE) {
 ff_mov_cenc_write_sinf_tag(track, pb, mov->encryption_kid);
 }
-- 
2.15.0


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

Re: [FFmpeg-devel] [PATCH RFC] libavdevice/decklink: Add support for EIA-708 output over SDI

[Dave Rice]

> On Oct 6, 2017, at 5:31 PM, Devin Heitmueller <dheitmuel...@ltnglobal.com> 
> wrote:
> 
>> Sorry, what I meant was:
>> Nothing inside FFmpeg except the decklink device could use
>> VANC?
> 
> Ah, I understand now.
> 
> Yes, the decklink device is currently the only SDI device which is supported 
> by libavdevice.  I’ve got a whole pile of patches coming which add support 
> for a variety of protocols for both capture and output (e.g. EIA-708, 
> SCTE-104, AFD, SMPTE 2038, etc).  But today the decklink module is the only 
> device supported.
> 
> Would love to see more SDI devices supported and potentially interested in 
> adding such support myself if we can find good candidates.  The DVEO/linsys 
> cards are largely obsolete and the AJA boards are significantly more 
> expensive than any of BlackMagic’s cards.  If anyone has good experiences 
> with other vendors I would like to hear about it (and there may be an 
> opportunity to extend libavdevice to support another SDI vendor).

The President of AJA has publicly stated an intent to add an open license to 
their SDK, https://twitter.com/ajaprez/status/910100436224499713 
<https://twitter.com/ajaprez/status/910100436224499713>. I’m glad to hear that 
handling other VANC data is in the works, I’m particularly interested in VITC 
and EIA-608 with inputs.

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

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

> On Oct 18, 2017, at 3:07 PM, Marton Balint <c...@passwd.hu> wrote:
> 
> On Mon, 16 Oct 2017, Dave Rice wrote:
> 
>> Hi,
>> 
>> I tested this with my Ultrastudio Express and confirmed that I'm getting 
>> higher bit depth recordings with the abitscope filter. This patch adds an 
>> option to get 32 bit audio as an input with the decklink device (beforehand 
>> only 16 bit audio was supported). This resolves 
>> http://trac.ffmpeg.org/ticket/6708 and is partly based upon Georg 
>> Lippitisch's earlier draft at 
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.
>> 
>> 
>> From fbf2bd40471c8fa35374bb1a51c51a3f4f36b992 Mon Sep 17 00:00:00 2001
>> From: Dave Rice <d...@dericed.com>
>> Date: Thu, 12 Oct 2017 13:40:59 -0400
>> Subject: [PATCH] libavdevice/decklink: 32 bit audio support
>> 
>> Partially based upon Georg Lippitsch's patch at 
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.
>> ---
>> libavdevice/decklink_common.h   |  1 +
>> libavdevice/decklink_common_c.h |  1 +
>> libavdevice/decklink_dec.cpp| 17 ++---
>> libavdevice/decklink_dec_c.c|  1 +
>> 4 files changed, 17 insertions(+), 3 deletions(-)
> 
> Missing docs/indevs.texi update and libavdevice micro bump.

Updated.

From 1e5ff78fec9b13eccac9a96acc358bbfd6a7015d Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Wed, 18 Oct 2017 15:21:46 -0400
Subject: [PATCH] libavdevice/decklink: 32 bit audio support

---
 doc/indevs.texi |  4 
 libavdevice/decklink_common.h   |  1 +
 libavdevice/decklink_common_c.h |  1 +
 libavdevice/decklink_dec.cpp| 17 ++---
 libavdevice/decklink_dec_c.c|  1 +
 libavdevice/version.h   |  2 +-
 6 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index 55a4084bb2..d308bbf7de 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -311,6 +311,10 @@ Sets maximum input buffer size in bytes. If the buffering 
reaches this value,
 incoming frames will be dropped.
 Defaults to @samp{1073741824}.
 
+@item audio_depth
+Sets the audio sample bit depth. Must be @samp{16} or @samp{32}.
+Defaults to @samp{16}.
+
 @end table
 
 @subsection Examples
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 6b2525fb53..b6acb01bb9 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -97,6 +97,7 @@ struct decklink_ctx {
 int frames_buffer_available_spots;
 
 int channels;
+int audio_depth;
 };
 
 typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index 5616ab32f9..368ac259e4 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -42,6 +42,7 @@ struct decklink_cctx {
 double preroll;
 int v210;
 int audio_channels;
+int audio_depth;
 int duplex_mode;
 DecklinkPtsSource audio_pts_source;
 DecklinkPtsSource video_pts_source;
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index d9ac01ac91..7e97d5f064 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -771,7 +771,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
 av_init_packet();
 
 //hack among hacks
-pkt.size = audioFrame->GetSampleFrameCount() * 
ctx->audio_st->codecpar->channels * (16 / 8);
+pkt.size = audioFrame->GetSampleFrameCount() * 
ctx->audio_st->codecpar->channels * (ctx->audio_depth / 8);
 audioFrame->GetBytes();
 audioFrame->GetPacketTime(_pts, ctx->audio_st->time_base.den);
 pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, 
ctx->audio_pts_source, ctx->audio_st->time_base, _audio_pts);
@@ -854,6 +854,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 ctx->audio_pts_source = cctx->audio_pts_source;
 ctx->video_pts_source = cctx->video_pts_source;
 ctx->draw_bars = cctx->draw_bars;
+ctx->audio_depth = cctx->audio_depth;
 cctx->ctx = ctx;
 
 /* Check audio channel option for valid values: 2, 8 or 16 */
@@ -866,6 +867,16 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 av_log(avctx, AV_LOG_ERROR, "Value of channels option must be one 
of 2, 8 or 16\n");
 return AVERROR(EINVAL);
 }
+
+/* Check audio bit depth option for valid values: 16 or 32 */
+switch (cctx->audio_depth) {
+case 16:
+case 32:
+break;
+default:
+av_log(avctx, AV_LOG_ERROR, "Value for audio bit depth option must 
be either 16 or 32\n");
+return AVERROR(EINVAL);
+}
 
 /* Lis

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

> On Oct 17, 2017, at 3:30 PM, Douglas Marsh <ffm...@dx9s.net> wrote:
> 
> On 2017-10-17 09:10, Dave Rice wrote:
> 
>>>> -audio_depth   .D.. audio bitdepth (from 0 to 1)
>>>> (default 16bits)
>>>>16bits   .D..
> 
>>> Hmm, first patch might be enough.
>> Sounds good to me. Unless anyone prefers "-audio_depth thirtytwo" :-D
>> Dave Rice
> 
> Yeah that works.. so if they have any other depths, can go 0, 1 or 2 (2=some 
> new bit depth yet to be created)
> 
> And for clarification: yes 32-bits (PCM_S32LE) -- I was just pointing out the 
> ADC/DAC's are 24-bit (8-bits padded).

I was suggesting `-audio_depth thirtytwo` in jest. IMHO assigning enumerated 
index numbers to purely numerical values will be confusing. For instance if in 
the future 12 bit is added then 24 bit, we'll have 
-audio_depth   .D.. audio bitdepth (from 0 to 3) (default 
16bits)
   16bits [0]
   32bits [1]
   12bits [2]
   24bits [3]

The alternative patch in the "decklink 24/32 bit question" thread, changes the 
default behavior of the decklink input which I think should be avoided. I agree 
with Moritz that the  first patch of this thread 
(https://patchwork.ffmpeg.org/patch/5588/) is the best option. Also the method 
used in the patch to validate a limited non-consecutive range of values is 
already in used elsewhere in the device for the channel count at 
https://github.com/FFmpeg/FFmpeg/blob/278588cd0bf788df0194f74e62745f68559616f9/libavdevice/decklink_dec.cpp#L859-L868.

Best Regards,

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

Re: [FFmpeg-devel] decklink 24/32 bit question

[Dave Rice]

> On Oct 17, 2017, at 2:59 PM, Moritz Barsnick <barsn...@gmx.net> wrote:
> 
> Hi Doug,
> 
> On Tue, Oct 03, 2017 at 20:39:49 -0700, Douglas Marsh wrote:
>> After digging around in places, made the following changes:
> [...]
>> It doesn't work (the audio capture is close but wrong), but believe it 
>> is a step in the correct direction. Anybody have a clue? Saw several 
>> names in cpp,c,h files including: Ramiro Polla, Luca Barbato, Deti 
>> Fliegl, Rafaël Carré and Akamai Technologies Inc.
> 
> Did you check out Dave Rice's recent patch (on this list)? It touches
> code in a few more places, and adds an option to select 16 vs. 32 bits.
> Please test, if you can.
> 
> Is your subject indicating that 24 bits depth could also be supported?
> If so, Dave perhaps should expand his patch to cover that.

The decklink sdk only defines two BMDAudioSampleType values: 
bmdAudioSampleType16bitInteger and bmdAudioSampleType32bitInteger. I don't 
think there's an easy way to support a 24 bit input here. Generally in this 
case I've used bmdAudioSampleType32bitInteger and then encode it at pcm_s24le.
Dave Rice

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

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

> On Oct 17, 2017, at 11:48 AM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 10/17/17, Moritz Barsnick <barsn...@gmx.net> wrote:
>> On Tue, Oct 17, 2017 at 11:12:46 -0400, Dave Rice wrote:
>> 
>>> Thanks for pointing me in the right direction. I am attaching an
>>> updated patch below; however, after removing my 16/32 and using the
>>> enumeration method (with the patch below), it accepts any value
>>> between AUDIO_BITDEPTH_LOWEST and AUDIO_BITDEPTH_NB-1 (16 and 32).
>>> When I enter any value from 17 through 31 then the output is still in
>>> pcm_s16le, however the resulting audio is slow and choppy (I'm
>>> guessing from the pkt.size line). Am I missing some method to say
>>> that the enumeration list is restrictive?
>> 
>> Yes, that was exactly the oversight I had. The const'ified option now
>> takes those strings, but also numbers within the range. So the option
>> value "16" will be either "16" (string) or 16 (or integer), "17" will
>> be interpreted as integer 17 (and still valid thanks to the range), and
>> so on. My bad.
>> 
>> So I guess it's stupid to use a string enum to represent a number
>> value. You could properly restrict its use to the enumerated values if
>> they were subsequent (i.e. 0, 1), and perhaps strings with letters for
>> the enumerated options. But that's totally unintuitive. It would result
>> in (auto doc):
>> 
>>  -audio_depth   .D.. audio bitdepth (from 0 to 1)
>> (default 16bits)
>> 16bits   .D..
>> 32bits   .D..
>> 
>> and you could use it with either
>>  -audio_depth 0 # 16 bits
>>  -audio_depth 1 # 32 bits
>>  -audio_depth 16bits # 16 bits
>>  -audio_depth 32bits # 16 bits
>> but not
>>  -audio_depth 16
>> 
>> Horrible!
>> 
>> Your original version was most intuitive, IMHO. Sorry for explaining
>> the concept. ;-) It fits well for anything representable by strings,
>> not numbers.
> 
> Hmm, first patch might be enough.

Sounds good to me. Unless anyone prefers "-audio_depth thirtytwo" :-D
Dave Rice


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

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

Hi Moritz,

> On Oct 17, 2017, at 10:40 AM, Moritz Barsnick <barsn...@gmx.net> wrote:
> 
> On Tue, Oct 17, 2017 at 09:22:27 -0400, Dave Rice wrote:
>>>> --- a/libavdevice/decklink_dec_c.c
>>>> +++ b/libavdevice/decklink_dec_c.c
>>>> @@ -72,6 +72,7 @@ static const AVOption options[] = {
>>>>{ "wallclock", NULL,  0,
>>>> AV_OPT_TYPE_CONST, { .i64 = PTS_SRC_WALLCLOCK}, 0, 0, DEC, "pts_source"},
>>>>{ "draw_bars", "draw bars on signal loss" , OFFSET(draw_bars),
>>>> AV_OPT_TYPE_BOOL,  { .i64 = 1}, 0, 1, DEC },
>>>>{ "queue_size","input queue buffer size",   OFFSET(queue_size),
>>>> AV_OPT_TYPE_INT64, { .i64 = (1024 * 1024 * 1024)}, 0, INT64_MAX, DEC },
>>>> +{ "audio_depth",   "audio bitdepth (16 or 32)", OFFSET(audio_depth),
>>>> AV_OPT_TYPE_INT,   { .i64 = 16}, 16, 32, DEC },
>>> 
>>> Use AV_OPT_ ENUM instead.
>> 
>> I don't see there to be an AV_OPT_TYPE_ENUM. Could you point out how this 
>> works from an example? Alternatively since the decklink sdk only supports 2 
>> bit depths (16 and 32), I could use a boolean here if that makes more sense.
> 
> I would guess Paul means:
> 
> enum audio_bitdepths {
>AUDIO_BITDEPTH_16,
>AUDIO_BITDEPTH_32,
>AUDIO_BITDEPTH_NB
> };
> 
>{ "audio_depth","audio bitdepth", OFFSET(audio_depth), 
> AV_OPT_TYPE_INT,   { .i64 = AUDIO_BITDEPTH_16 }, 0, AUDIO_BITDEPTH_NB-1, DEC, 
> "audio_depth" },
>{ "16", "16 bits",  0, 
> AV_OPT_TYPE_CONST, { .i64 = AUDIO_BITDEPTH_16 }, 0, 0,   DEC, 
> "audio_depth" },
>{ "32", "32 bits",  0, 
> AV_OPT_TYPE_CONST, { .i64 = AUDIO_BITDEPTH_32 }, 0, 0,   DEC, 
> "audio_depth" },
> 
> I'm not sure pure digits are accepted as an option strin ("16", "32"), but, 
> why shouldn't they.
> 
> It may seem like a bit of overkill for two values, but take into
> consideration that you can skip the check "Check audio bit depth option
> for valid values: 16 or 32", but need to map the enums to values
> instead. Or you pre-assign the enums values (AUDIO_BITDEPTH_16 = 16,
> ...), but use some other trick to define the lowest possible value.[*]
> And you can easily add 8 bit or 24 bit support. ;-)
> 
> Cheers,
> Moritz
> 
> [*]
> enum audio_bitdepths {
>AUDIO_BITDEPTH_LOWEST = 16,
>AUDIO_BITDEPTH_16 = 16,
>AUDIO_BITDEPTH_32 = 32,
>AUDIO_BITDEPTH_NB
> };
> 
>{ "audio_depth","audio bitdepth", OFFSET(audio_depth), 
> AV_OPT_TYPE_INT,   { .i64 = AUDIO_BITDEPTH_16 }, AUDIO_BITDEPTH_LOWEST, 
> AUDIO_BITDEPTH_NB-1, DEC, "audio_depth" },
>{ "16", "16 bits",  0, 
> AV_OPT_TYPE_CONST, { .i64 = AUDIO_BITDEPTH_16 }, 0, 0, DEC, "audio_depth" },
>{ "32", "32 bits",  0, 
> AV_OPT_TYPE_CONST, { .i64 = AUDIO_BITDEPTH_32 }, 0, 0, DEC, "audio_depth" },
> ___

Thanks for pointing me in the right direction. I am attaching an updated patch 
below; however, after removing my 16/32 and using the enumeration method (with 
the patch below), it accepts any value between AUDIO_BITDEPTH_LOWEST and 
AUDIO_BITDEPTH_NB-1 (16 and 32). When I enter any value from 17 through 31 then 
the output is still in pcm_s16le, however the resulting audio is slow and 
choppy (I'm guessing from the pkt.size line). Am I missing some method to say 
that the enumeration list is restrictive?


From 10f3003e8b690bba1a722b615481d80644cdb2c1 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Tue, 17 Oct 2017 10:52:10 -0400
Subject: [PATCH] libavdevice/decklink: 32 bit audio support

---
 libavdevice/decklink_common.h   | 1 +
 libavdevice/decklink_common_c.h | 8 
 libavdevice/decklink_dec.cpp| 7 ---
 libavdevice/decklink_dec_c.c| 3 +++
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 6b2525fb53..b6acb01bb9 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -97,6 +97,7 @@ struct decklink_ctx {
 int frames_buffer_available_spots;
 
 int channels;
+int audio_depth;
 };
 
 typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
diff --git a/libavdevice/decklink_co

Re: [FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

Hi Paul,

> On Oct 16, 2017, at 3:56 PM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 10/16/17, Dave Rice <d...@dericed.com <mailto:d...@dericed.com>> wrote:
>> Hi,
>> 
>> I tested this with my Ultrastudio Express and confirmed that I'm getting
>> higher bit depth recordings with the abitscope filter. This patch adds an
>> option to get 32 bit audio as an input with the decklink device (beforehand
>> only 16 bit audio was supported). This resolves
>> http://trac.ffmpeg.org/ticket/6708 and is partly based upon Georg
>> Lippitisch's earlier draft at
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.
>> 
>> 
>> From fbf2bd40471c8fa35374bb1a51c51a3f4f36b992 Mon Sep 17 00:00:00 2001
>> From: Dave Rice <d...@dericed.com>
>> Date: Thu, 12 Oct 2017 13:40:59 -0400
>> Subject: [PATCH] libavdevice/decklink: 32 bit audio support
>> 
>> Partially based upon Georg Lippitsch's patch at
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.
>> ---
>> libavdevice/decklink_common.h   |  1 +
>> libavdevice/decklink_common_c.h |  1 +
>> libavdevice/decklink_dec.cpp| 17 ++---
>> libavdevice/decklink_dec_c.c|  1 +
>> 4 files changed, 17 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
>> index 6b2525fb53..b6acb01bb9 100644
>> --- a/libavdevice/decklink_common.h
>> +++ b/libavdevice/decklink_common.h
>> @@ -97,6 +97,7 @@ struct decklink_ctx {
>> int frames_buffer_available_spots;
>> 
>> int channels;
>> +int audio_depth;
>> };
>> 
>> typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
>> diff --git a/libavdevice/decklink_common_c.h
>> b/libavdevice/decklink_common_c.h
>> index 5616ab32f9..368ac259e4 100644
>> --- a/libavdevice/decklink_common_c.h
>> +++ b/libavdevice/decklink_common_c.h
>> @@ -42,6 +42,7 @@ struct decklink_cctx {
>> double preroll;
>> int v210;
>> int audio_channels;
>> +int audio_depth;
>> int duplex_mode;
>> DecklinkPtsSource audio_pts_source;
>> DecklinkPtsSource video_pts_source;
>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> index d9ac01ac91..7e97d5f064 100644
>> --- a/libavdevice/decklink_dec.cpp
>> +++ b/libavdevice/decklink_dec.cpp
>> @@ -771,7 +771,7 @@ HRESULT
>> decklink_input_callback::VideoInputFrameArrived(
>> av_init_packet();
>> 
>> //hack among hacks
>> -pkt.size = audioFrame->GetSampleFrameCount() *
>> ctx->audio_st->codecpar->channels * (16 / 8);
>> +pkt.size = audioFrame->GetSampleFrameCount() *
>> ctx->audio_st->codecpar->channels * (ctx->audio_depth / 8);
>> audioFrame->GetBytes();
>> audioFrame->GetPacketTime(_pts,
>> ctx->audio_st->time_base.den);
>> pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock,
>> ctx->audio_pts_source, ctx->audio_st->time_base, _audio_pts);
>> @@ -854,6 +854,7 @@ av_cold int ff_decklink_read_header(AVFormatContext
>> *avctx)
>> ctx->audio_pts_source = cctx->audio_pts_source;
>> ctx->video_pts_source = cctx->video_pts_source;
>> ctx->draw_bars = cctx->draw_bars;
>> +ctx->audio_depth = cctx->audio_depth;
>> cctx->ctx = ctx;
>> 
>> /* Check audio channel option for valid values: 2, 8 or 16 */
>> @@ -866,6 +867,16 @@ av_cold int ff_decklink_read_header(AVFormatContext
>> *avctx)
>> av_log(avctx, AV_LOG_ERROR, "Value of channels option must be
>> one of 2, 8 or 16\n");
>> return AVERROR(EINVAL);
>> }
>> +
>> +/* Check audio bit depth option for valid values: 16 or 32 */
>> +switch (cctx->audio_depth) {
>> +case 16:
>> +case 32:
>> +break;
>> +default:
>> +av_log(avctx, AV_LOG_ERROR, "Value for audio bit depth option
>> must be either 16 or 32\n");
>> +return AVERROR(EINVAL);
> 
> This message is redundant with AVOptions, User should be able to
> select only 16 or 32,
> see bellow.
> 
>> +}
>> 
>> /* List available devices. */
>> if (ctx->list_devices) {
>> @@ -930,7 +941,7 @@ av_cold int ff_decklink_read_header(AVFormatContext
>> *avctx)
>> goto error;
>> }
>> st->codecpar->codec_type  = AVMEDIA_T

[FFmpeg-devel] [PATCH] libavdevice/decklink: 32 bit audio support

[Dave Rice]

Hi,

I tested this with my Ultrastudio Express and confirmed that I'm getting higher 
bit depth recordings with the abitscope filter. This patch adds an option to 
get 32 bit audio as an input with the decklink device (beforehand only 16 bit 
audio was supported). This resolves http://trac.ffmpeg.org/ticket/6708 and is 
partly based upon Georg Lippitisch's earlier draft at 
https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.


From fbf2bd40471c8fa35374bb1a51c51a3f4f36b992 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 12 Oct 2017 13:40:59 -0400
Subject: [PATCH] libavdevice/decklink: 32 bit audio support

Partially based upon Georg Lippitsch's patch at 
https://ffmpeg.org/pipermail/ffmpeg-devel/2015-January/167649.html.
---
 libavdevice/decklink_common.h   |  1 +
 libavdevice/decklink_common_c.h |  1 +
 libavdevice/decklink_dec.cpp| 17 ++---
 libavdevice/decklink_dec_c.c|  1 +
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 6b2525fb53..b6acb01bb9 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -97,6 +97,7 @@ struct decklink_ctx {
 int frames_buffer_available_spots;
 
 int channels;
+int audio_depth;
 };
 
 typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index 5616ab32f9..368ac259e4 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -42,6 +42,7 @@ struct decklink_cctx {
 double preroll;
 int v210;
 int audio_channels;
+int audio_depth;
 int duplex_mode;
 DecklinkPtsSource audio_pts_source;
 DecklinkPtsSource video_pts_source;
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index d9ac01ac91..7e97d5f064 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -771,7 +771,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
 av_init_packet();
 
 //hack among hacks
-pkt.size = audioFrame->GetSampleFrameCount() * 
ctx->audio_st->codecpar->channels * (16 / 8);
+pkt.size = audioFrame->GetSampleFrameCount() * 
ctx->audio_st->codecpar->channels * (ctx->audio_depth / 8);
 audioFrame->GetBytes();
 audioFrame->GetPacketTime(_pts, ctx->audio_st->time_base.den);
 pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, 
ctx->audio_pts_source, ctx->audio_st->time_base, _audio_pts);
@@ -854,6 +854,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 ctx->audio_pts_source = cctx->audio_pts_source;
 ctx->video_pts_source = cctx->video_pts_source;
 ctx->draw_bars = cctx->draw_bars;
+ctx->audio_depth = cctx->audio_depth;
 cctx->ctx = ctx;
 
 /* Check audio channel option for valid values: 2, 8 or 16 */
@@ -866,6 +867,16 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 av_log(avctx, AV_LOG_ERROR, "Value of channels option must be one 
of 2, 8 or 16\n");
 return AVERROR(EINVAL);
 }
+
+/* Check audio bit depth option for valid values: 16 or 32 */
+switch (cctx->audio_depth) {
+case 16:
+case 32:
+break;
+default:
+av_log(avctx, AV_LOG_ERROR, "Value for audio bit depth option must 
be either 16 or 32\n");
+return AVERROR(EINVAL);
+}
 
 /* List available devices. */
 if (ctx->list_devices) {
@@ -930,7 +941,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
 goto error;
 }
 st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
-st->codecpar->codec_id= AV_CODEC_ID_PCM_S16LE;
+st->codecpar->codec_id= cctx->audio_depth == 32 ? 
AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
 st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
 st->codecpar->channels= cctx->audio_channels;
 avpriv_set_pts_info(st, 64, 1, 100);  /* 64 bits pts in us */
@@ -1021,7 +1032,7 @@ av_cold int ff_decklink_read_header(AVFormatContext 
*avctx)
 }
 
 av_log(avctx, AV_LOG_VERBOSE, "Using %d input audio channels\n", 
ctx->audio_st->codecpar->channels);
-result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz, 
bmdAudioSampleType16bitInteger, ctx->audio_st->codecpar->channels);
+result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz, 
cctx->audio_depth == 32 ? bmdAudioSampleType32bitInteger : 
bmdAudioSampleType16bitInteger, ctx->audio_st->codecpar->channels);
 
 if (result != S_OK) {
 av_log(avctx, AV_LOG_ERROR, "Cannot enable audio input\n");
diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
index 1127d23ada..1c6d82694

[FFmpeg-devel] [PATCH] libavcodec: fix field_order labelling

[Dave Rice]

Hello all,
This issue originated in this thread 
https://github.com/amiaopensource/vrecord/issues/170. On Field Order, in the 
QuickTime specification at 
https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html
 (and similarly in the Matroska specification which adopted similar language) 
it states the following meanings for field order values:

> 9 – B is displayed earliest, T is stored first in the file. 14 – T is 
> displayed earliest, B is stored first in the file.

 This definition is contradicted by other Apple documentation such as 
https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION.
 

An Apple engineer confirmed that the QuickTime specification’s definitions for 
those Field Order values is wrong and does not match Apple’s (of FFmpeg’s) 
practice, see 
https://github.com/amiaopensource/vrecord/issues/170#issuecomment-321937668.

However I think that some of the commenting in ffmpeg is based upon the 
inaccurate definitions from Apple. For instance, in that thread David Singer 
confirms:

> Ah, not quite. 1 and 6 are indeed 'planar' (all of one field before all of 
> the other). They don't concern us. Both 9 and 14 are stored in spatial order 
> (i.e. you could do terrible de-interlacing by simply displaying the buffer as 
> a frame), and the 9 or 14 value tells you which field is to be displayed 
> first.
> 
> 9 – T is earlier than B. 14 – B is earlier than T

mov.c associates AV_FIELD_TB with 9 and AV_FIELD_BT with 14 (similar 
associations in matroska.h), but avcodec.h states:

> AV_FIELD_TB,  //< Top coded first, bottom displayed first
> AV_FIELD_BT,  //< Bottom coded first, bottom displayed first

IMHO in both cases of AV_FIELD_TB and AV_FIELD_BT the coding should be referred 
as interleaved rather than ‘bottom coded first’ or ‘top coded first’. In the 
case of AV_FIELD_TT and AV_FIELD_BB the fields are stored as planar images 
where storage order is notable, but with TB and BT the fields are interleaved.

Also utils.c associates these field order values with the following labels:

> AV_FIELD_TB  -> "top coded first (swapped)";
> AV_FIELD_BT -> "bottom coded first (swapped)";

From my reading, I infer that "top coded first (swapped)” means "top coded 
first, bottom displayed first”; however in practice from files generated by 
QuickTime and FFmpeg files with a value of TB have the top field displayed 
first, so I think the labels are swapped. In the patch below I suggest using 
“top first (interleaved)” for TB and “bottom first (interleaved)” for BT.

Comments?


From de871b3fa891fa0ae6856461c1f8305cc889cde7 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Sat, 12 Aug 2017 12:30:43 -0400
Subject: [PATCH] libavcodec: fix field_order labelling

---
 libavcodec/avcodec.h | 4 ++--
 libavcodec/utils.c   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c594993766..37c39072b3 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1726,8 +1726,8 @@ enum AVFieldOrder {
 AV_FIELD_PROGRESSIVE,
 AV_FIELD_TT,  //< Top coded_first, top displayed first
 AV_FIELD_BB,  //< Bottom coded first, bottom displayed first
-AV_FIELD_TB,  //< Top coded first, bottom displayed first
-AV_FIELD_BT,  //< Bottom coded first, top displayed first
+AV_FIELD_TB,  //< Interleaved coding, top displayed first
+AV_FIELD_BT,  //< Interleaved coding, bottom displayed first
 };
 
 /**
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 1336e921c9..926c964846 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1406,9 +1406,9 @@ void avcodec_string(char *buf, int buf_size, 
AVCodecContext *enc, int encode)
 else if (enc->field_order == AV_FIELD_BB)
 field_order = "bottom first";
 else if (enc->field_order == AV_FIELD_TB)
-field_order = "top coded first (swapped)";
+field_order = "top first (interleaved)";
 else if (enc->field_order == AV_FIELD_BT)
-field_order = "bottom coded first (swapped)";
+field_order = "bottom first (interleaved)";
 
 av_strlcatf(detail, sizeof(detail), "%s, ", field_order);
 }
-- 
2.13.1


Thanks much,
Dave Rice


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix setting field_order during muxing

[Dave Rice]

> On Aug 10, 2017, at 5:18 PM, Jerome Martinez <jer...@mediaarea.net> wrote:
> 
> Le 10/08/2017 à 04:43, James Almer a écrit :
>> AVFrame.top_field_first doxy states
>> 
>> "If the content is interlaced, is top field displayed first."
>> 
>> And AVFieldOrder doxy defines:
>> AV_FIELD_TB,  //< Top coded first, bottom displayed first
>> AV_FIELD_BT,  //< Bottom coded first, top displayed first
>> 
>> Fixes ticket #6577
> 
> IMHO the subject is complex, and everyone should be in sync with the purpose 
> of each mux_par->field_order value and understand impact on other players 
> (sometimes the MOV metadata is the only one available as the codec has no 
> info e.g. uncompressed or jp2k) before changing current behavior in a so 
> general manner.
> What I understood is that it is based on QuickTime specs (the mapping is 1 to 
> 1 from fiel atom) and TN2162 and that they are not easy to understand and 
> maybe misleading : the feedback I got in the last years about the meaning of 
> this atom are not really in sync with specs having stored vs displayed, and 
> such change may have an impact on how other players handle fiel atom or other 
> metadata in the way it is currently implemented by FFmpeg.

I filed ticket #6577 but not feel cautious about this patch and consider that 
my initial report may have been based on some misunderstanding. As Jerome notes 
the MKV field order flag is derived from mov's fiel atom and there are two 
definitions for that:

https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html
 (Table 4-2)

https://developer.apple.com/library/content/technotes/tn2162/_index.html#//apple_ref/doc/uid/DTS40013070-CH1-TNTAG10-THE__FIEL__IMAGEDESCRIPTION_EXTENSION__FIELD_FRAME_INFORMATION
 (Figure 4)

From testing we have found that ffmpeg's behavior here is matching some Apple 
mov muxers in the assignment of a field order value, so the patch may be 
unneeded or the reason may need more testing.

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

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Dave Rice]

> On Jul 7, 2017, at 7:06 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 7/7/2017 10:13 PM, James Almer wrote:
>> Isn't this necessary only for files with raw video? As is, this box
>> would be written on any mov file with a video stream.
> 
> This was addressed a previous email:
> 
>http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html
> 
> I guess the spec is up for interpretation.

The quicktime spec says "This is a mandatory extension for all uncompressed 
Y´CbCr data formats”. It doesn’t clarify if the clap atom is recommended or not 
in mov files that are not “uncompressed Y´CbCr”, though it would make sense if 
the video container needs to store cropping data. I think constraining the 
change for only  “uncompressed Y´CbCr” would be more cautious though. I’ll 
revise my patch to include the condition and resubmit.

If the patch only impacts “uncompressed Y´CbCr” would any fate updates be 
needed?
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] movenc: write clap tag

[Dave Rice]

> On Jul 7, 2017, at 11:32 AM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 7/7/2017 3:20 AM, Dave Rice wrote:
>> Resolves https://trac.ffmpeg.org/ticket/6145 and writes a clap atom that is 
>> coincident with the frames width and height.
>> 
>> 
>> From 23d80d0d47829fed61e817b1e7c3f6d420c9ab5c Mon Sep 17 00:00:00 2001
>> From: Dave Rice <d...@dericed.com>
>> Date: Thu, 6 Jul 2017 21:12:38 -0400
>> Subject: [PATCH] movenc: write clap tag
>> 
>> ---
>> libavformat/movenc.c | 19 +++
>> 1 file changed, 19 insertions(+)
> 
> Whoever pushes, add the ticket number into the commit message.
> 
> [...]
> 
>> +avio_wb32(pb, 0); /* horizOff_N (= 0) */
>> +avio_wb32(pb, 1); /* horizOff_D (= 1) */
>> +avio_wb32(pb, 0); /* vertOff_N (= 0) */
>> +avio_wb32(pb, 1); /* vertOff_D (= 1) */
> 
> Near as I can tell, these related the active image area? Is it useful to ever 
> set
> this to anything but 0 (aka the whole image is active)?

Usually not a good idea to set a denominator to zero. The clap atom 
documentation at 
https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html#//apple_ref/doc/uid/TP4939-CH205-125850
 
<https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html#//apple_ref/doc/uid/TP4939-CH205-125850>
 specifies zero for numerators and one for denominators as default when the 
ratio of an actual aperture is not used.

> Was thinking of stuff like
> analogue video, a la http://chromashift.org/aspectratio/. I assume the answer 
> is
> probably 'no’.

The atom is analogous to the PixelCrop* elements of Matroska. For instance a 
standard definition NTSC video may typically be digitized to a frame size of 
720x486 but the active image might only be 704x480, so the aperture atom could 
express the difference.

> Patch itself LGTM.

[…] 

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

[FFmpeg-devel] [PATCH] movenc: write clap tag

[Dave Rice]

Resolves https://trac.ffmpeg.org/ticket/6145 and writes a clap atom that is 
coincident with the frames width and height.


From 23d80d0d47829fed61e817b1e7c3f6d420c9ab5c Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 6 Jul 2017 21:12:38 -0400
Subject: [PATCH] movenc: write clap tag

---
 libavformat/movenc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 88f2f2c819..31a5de8d5c 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1669,6 +1669,21 @@ static int mov_write_sv3d_tag(AVFormatContext *s, 
AVIOContext *pb, AVSphericalMa
 return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+avio_wb32(pb, 40);
+ffio_wfourcc(pb, "clap");
+avio_wb32(pb, track->par->width); /* apertureWidth_N */
+avio_wb32(pb, 1); /* apertureWidth_D (= 1) */
+avio_wb32(pb, track->height); /* apertureHeight_N */
+avio_wb32(pb, 1); /* apertureHeight_D (= 1) */
+avio_wb32(pb, 0); /* horizOff_N (= 0) */
+avio_wb32(pb, 1); /* horizOff_D (= 1) */
+avio_wb32(pb, 0); /* vertOff_N (= 0) */
+avio_wb32(pb, 1); /* vertOff_D (= 1) */
+return 40;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1939,6 +1954,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format 
is not MOV or MP4.\n");
 }
 
+if (track->mode == MODE_MOV) {
+mov_write_clap_tag(pb, track);
+}
+
 if (track->mode == MODE_MP4 && mov->fc->strict_std_compliance <= 
FF_COMPLIANCE_UNOFFICIAL) {
 AVStereo3D* stereo_3d = (AVStereo3D*) 
av_stream_get_side_data(track->st, AV_PKT_DATA_STEREO3D, NULL);
 AVSphericalMapping* spherical_mapping = 
(AVSphericalMapping*)av_stream_get_side_data(track->st, AV_PKT_DATA_SPHERICAL, 
NULL);
-- 
2.13.0


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

Re: [FFmpeg-devel] [RFC][PATCH] movenc: write clap tag

[Dave Rice]

> On Jul 6, 2017, at 9:39 PM, Derek Buitenhuis <derek.buitenh...@gmail.com> 
> wrote:
> 
> On 7/7/2017 2:20 AM, Dave Rice wrote:
>> Currently this patch writes the clap atom under this condition 
>> "track->par->codec_id == AV_CODEC_ID_RAWVIDEO & track->mode == MODE_MOV”; 
>> however, AV_CODEC_ID_RAWVIDEO isn’t quite what the spec asks for. Any advice 
>> on how to write this condition for “uncompressed Y´CbCr data formats”; for 
>> instance v210 is considered uncompress Y´CbCr in QuickTime but is not a 
>> AV_CODEC_ID_RAWVIDEO. I haven’t found a concise way to add a condition for 
>> uncompressed YUV.
> 
> First, it should be '&&', I assume.

Thanks.

> I don't think there's any concise way of checking such conditions... you'll
> probably have to do something like a list of "codes" (v410, v210, raw) and
> some sort of sanity check for track->par->format to confirm is is YCbCr.
> Maybe someone knows better than myself.

I see that although it’s mandatory for uncompressed YUV, there’s no constraint 
to not use it otherwise, so I think I’ll proceed with just a check for 
MODE_MOV. I just checked ffmbc’s movenc.c and it also only checks for MODE_MOV.
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [RFC][PATCH] movenc: write clap tag

[Dave Rice]

Hi all,
I’m looking for some assistance on this patch which is intended to resolve 
https://trac.ffmpeg.org/ticket/6145 <https://trac.ffmpeg.org/ticket/6145>.

In 
https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html
 
<https://developer.apple.com/library/content/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html>,
 regarding the clap atom, it states "This is a mandatory extension for all 
uncompressed Y´CbCr data formats”. This patch is intended to write the clap 
atom in those cases as a coincident with the stored image (not supported a 
cropped aperature yet), but I’m having trouble associating it with the correct 
conditional formats. 

Currently this patch writes the clap atom under this condition 
"track->par->codec_id == AV_CODEC_ID_RAWVIDEO & track->mode == MODE_MOV”; 
however, AV_CODEC_ID_RAWVIDEO isn’t quite what the spec asks for. Any advice on 
how to write this condition for “uncompressed Y´CbCr data formats”; for 
instance v210 is considered uncompress Y´CbCr in QuickTime but is not a 
AV_CODEC_ID_RAWVIDEO. I haven’t found a concise way to add a condition for 
uncompressed YUV.

Other comments welcome. Thanks.

From a5e399c6eeaa64aef52dc498cc62114428f42941 Mon Sep 17 00:00:00 2001
From: Dave Rice <d...@dericed.com>
Date: Thu, 6 Jul 2017 21:12:38 -0400
Subject: [PATCH] movenc: write clap tag

---
 libavformat/movenc.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 88f2f2c819..033e8550b2 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1669,6 +1669,21 @@ static int mov_write_sv3d_tag(AVFormatContext *s, 
AVIOContext *pb, AVSphericalMa
 return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+avio_wb32(pb, 64);
+ffio_wfourcc(pb, "clap");
+avio_wb32(pb, track->par->width); /* aperatureWidth numerator */
+avio_wb32(pb, 1); /* aperatureWidth denominator (= 1) */
+avio_wb32(pb, track->height); /* aperatureHeight numerator */
+avio_wb32(pb, 1); /* aperatureHeight denominator (= 1) */
+avio_wb32(pb, 0); /* horizontal offset numerator (= 0) */
+avio_wb32(pb, 1); /* horizontal offset denominator (= 1) */
+avio_wb32(pb, 0); /* vertical offset numerator (= 0) */
+avio_wb32(pb, 1); /* vertical offset denominator (= 1) */
+return 64;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
 AVRational sar;
@@ -1939,6 +1954,10 @@ static int mov_write_video_tag(AVIOContext *pb, 
MOVMuxContext *mov, MOVTrack *tr
 av_log(mov->fc, AV_LOG_WARNING, "Not writing 'colr' atom. Format 
is not MOV or MP4.\n");
 }
 
+if (track->par->codec_id == AV_CODEC_ID_RAWVIDEO & track->mode == 
MODE_MOV) {
+mov_write_clap_tag(pb, track);
+}
+
 if (track->mode == MODE_MP4 && mov->fc->strict_std_compliance <= 
FF_COMPLIANCE_UNOFFICIAL) {
 AVStereo3D* stereo_3d = (AVStereo3D*) 
av_stream_get_side_data(track->st, AV_PKT_DATA_STEREO3D, NULL);
 AVSphericalMapping* spherical_mapping = 
(AVSphericalMapping*)av_stream_get_side_data(track->st, AV_PKT_DATA_SPHERICAL, 
NULL);
-- 
2.13.0

Best Regards,
Dave Rice

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

Re: [FFmpeg-devel] [WIP][PATCH] avfilter: add video oscilloscope filter

[Dave Rice]

> On Apr 26, 2017, at 2:37 AM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 4/26/17, Dave Rice <d...@dericed.com> wrote:
>> 
>> 
>> I'm curious why floats are used for x,y coordinates within a frame rather
>> than integers.
> 
> So it the position be set relatively. And for similar results for
> different resolutions.

Having been used to other filters with integers as x,y, this seems a bit 
unintuitive to me. In other filters relative positions are set by variables, 
such as ih/2 as opposed to 0.5. If staying with floats, I'd suggest that the 
documentation include examples for the use of calling specific lines by integer 
by including math in the options. Perhaps the statistical area could also state 
the x,y coords of the two ends of the scope as ints so that a viewer of the 
image would be clear to what line(s) are included in the analysis.

>> It appears that the plotted scope is padded to 300, though the trace usually
>> is 256 (2^8) lines tall. Are the other 44 lines intended to store something?
>> 
> 
> Yes, some statistics numbers.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avfilter: add pixscope filter

[Dave Rice]

> On Apr 24, 2017, at 5:03 PM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 4/24/17, Dave Rice <d...@dericed.com> wrote:
>> 
>>> On Apr 24, 2017, at 4:47 PM, Paul B Mahol <one...@gmail.com> wrote:
>>> 
>>> On 4/24/17, Dave Rice <d...@dericed.com> wrote:
>>>> 
>>>> I tested this filter and find it very useful, but could it be adjusted to
>>>> handle full range video. Currently when I test with a sample such as:
>>>> 
>>>> ./ffplay -f lavfi -i
>>>> color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope
>>>> 
>>>> The outputs list Y=235, U=240, V=16.
>>> 
>>> Hmm, I cannot reproduce this with my old ffplay build or latest mpv or
>>> latest ffmpeg.
>>> 
>>> So it must be ffplay issue.
>> 
>> Can you recreate it with:
>> 
>> ./ffmpeg -f lavfi -i
>> color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope -f
>> nut - | mpv -
> 
> No.
> 
> Neither with:
> 
> ./ffmpeg -f lavfi -i
> color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope
> -f nut -c:v rawvideo - | mpv -
> 
> or:
> 
> mpv 
> avdevice://lavfi:color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope
>  
> 

I hard reset my copy of the repo, re-applied the patch, and now it works as 
expected. Thanks.
Dave Rice

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

Re: [FFmpeg-devel] [WIP][PATCH] avfilter: add video oscilloscope filter

[Dave Rice]

 draw->pixelstep[0] + i] = color->comp[0].u8[i];
> +}
> +} else {
> +out->data[p][out->linesize[p] * (y0 >> draw->vsub[p]) + (x0 
> >> draw->hsub[p])] = color->comp[p].u8[0];
> +}
> +}
> +
> +if (x0 == x1 && y0 == y1)
> +break;
> +
> +e2 = err;
> +
> +if (e2 >-dx) {
> +err -= dy;
> +x0 += sx;
> +}
> +
> +if (e2 < dy) {
> +err += dx;
> +y0 += sy;
> +}
> +}
> +}
> +
> +static int oscilloscope_filter_frame(AVFilterLink *inlink, AVFrame *frame)
> +{
> +AVFilterContext *ctx  = inlink->dst;
> +OscilloscopeContext *s = ctx->priv;
> +AVFilterLink *outlink = ctx->outputs[0];
> +float average[4] = { 0 };
> +int max[4] = { 0 };
> +int min[4] = { INT_MAX, INT_MAX, INT_MAX, INT_MAX };
> +int i, c;
> +
> +s->nb_values = 0;
> +draw_scope(s, s->x1, s->y1, s->x2, s->y2, frame, s->values);
> +ff_blend_rectangle(>draw, >dark, frame->data, frame->linesize,
> +   frame->width, frame->height,
> +   s->ox, s->oy, s->width, 300);

It appears that the plotted scope is padded to 300, though the trace usually is 
256 (2^8) lines tall. Are the other 44 lines intended to store something?

> +if (s->grid) {
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox, s->oy, s->width - 1, 1);
> +
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox, s->oy + 64, s->width, 1);
> +
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox, s->oy + 128, s->width, 1);
> +
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox, s->oy + 192, s->width, 1);
> +
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox, s->oy + 256, s->width, 1);
> +
> +for (i = 0; i < 10; i++) {
> +ff_fill_rectangle(>draw, >gray, frame->data, 
> frame->linesize,
> +  s->ox + i * (s->width - 1) / 10, s->oy, 1, 
> 256);
> +}
> +
> +ff_fill_rectangle(>draw, >gray, frame->data, frame->linesize,
> +  s->ox + s->width - 1, s->oy, 1, 256);
> +}
> +
> +for (i = 1; i < s->nb_values; i++) {
> +for (c = 0; c < s->nb_comps; c++) {
> +if ((1 << c) & s->components) {
> +int x = i * s->width / s->nb_values;
> +int px = (i - 1) * s->width / s->nb_values;
> +int py = 255 - s->values[i-1].p[c];
> +int y = 255 - s->values[i].p[c];
> +
> +draw_line(>draw, s->ox + x, s->oy + y, s->ox + px, s->oy 
> + py, frame, s->colors[c]);
> +}
> +}
> +}
> +
> +return ff_filter_frame(outlink, frame);
> +}
> +
> +static const AVFilterPad oscilloscope_inputs[] = {
> +{
> +.name   = "default",
> +.type   = AVMEDIA_TYPE_VIDEO,
> +.filter_frame   = oscilloscope_filter_frame,
> +.config_props   = oscilloscope_config_input,
> +.needs_writable = 1,
> +},
> +{ NULL }
> +};
> +
> +static const AVFilterPad oscilloscope_outputs[] = {
> +{
> +.name = "default",
> +.type = AVMEDIA_TYPE_VIDEO,
> +},
> +{ NULL }
> +};
> +
> +AVFilter ff_vf_oscilloscope = {
> +.name  = "oscilloscope",
> +.description   = NULL_IF_CONFIG_SMALL("2D Video Oscilloscope."),
> +.priv_size = sizeof(OscilloscopeContext),
> +.priv_class= _class,
> +.query_formats = query_formats,
> +.uninit= oscilloscope_uninit,
> +.inputs= oscilloscope_inputs,
> +.outputs   = oscilloscope_outputs,
> +};
> -- 
> 2.9.3

A crash on >8 bit:

./ffplay -f lavfi -i mandelbrot -vf format=yuv422p10le,oscilloscope
ffplay version N-85657-gd2e300a0a0 Copyright (c) 2003-2017 the FFmpeg developers
  built with Apple LLVM version 8.0.0 (clang-800.0.42.1)
  configuration: --disable-dwt
  libavutil  55. 61.100 / 55. 61.100
  libavcodec 57. 93.100 / 57. 93.100
  libavformat57. 72.101 / 57. 72.101
  libavdevice57.  7.100 / 57.  7.100
  libavfilter 6. 87.100 /  6. 87.100
  libswscale  4.  7.101 /  4.  7.101
  libswresample   2.  8.100 /  2.  8.100
Input #0, lavfi, from 'mandelbrot':0KB vq=0KB sq=0B f=0/0   
  Duration: N/A, start: 0.00, bitrate: N/A
Stream #0:0: Video: rawvideo (RGB[0] / 0x424752), rgb0, 640x480 [SAR 1:1 
DAR 4:3], 25 tbr, 25 tbn, 25 tbc
Segmentation fault: 11


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

Re: [FFmpeg-devel] [PATCH] avfilter: add pixscope filter

[Dave Rice]

> On Apr 24, 2017, at 4:47 PM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 4/24/17, Dave Rice <d...@dericed.com> wrote:
>> 
>> I tested this filter and find it very useful, but could it be adjusted to
>> handle full range video. Currently when I test with a sample such as:
>> 
>> ./ffplay -f lavfi -i
>> color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope
>> 
>> The outputs list Y=235, U=240, V=16.
> 
> Hmm, I cannot reproduce this with my old ffplay build or latest mpv or
> latest ffmpeg.
> 
> So it must be ffplay issue.

Can you recreate it with:

./ffmpeg -f lavfi -i 
color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope -f nut 
- | mpv -

Dave

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

Re: [FFmpeg-devel] [PATCH] avfilter: add pixscope filter

[Dave Rice]

; +Y = 0;
> +}
> +
> +ff_blend_rectangle(>draw, >dark, in->data, in->linesize,
> +   in->width, in->height,
> +   X,
> +   Y,
> +   210,
> +   300);
> +
> +ff_blend_rectangle(>draw, >black, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 2, s->y - 2, s->w + 4, 1);
> +
> +ff_blend_rectangle(>draw, >white, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 1, s->y - 1, s->w + 2, 1);
> +
> +ff_blend_rectangle(>draw, >white, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 1, s->y - 1, 1, s->h + 2);
> +
> +ff_blend_rectangle(>draw, >black, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 2, s->y - 2, 1, s->h + 4);
> +
> +ff_blend_rectangle(>draw, >white, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 1, s->y + 1 + s->h, s->w + 3, 1);
> +
> +ff_blend_rectangle(>draw, >black, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x - 2, s->y + 2 + s->h, s->w + 4, 1);
> +
> +ff_blend_rectangle(>draw, >white, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x + 1 + s->w, s->y - 1, 1, s->h + 2);
> +
> +ff_blend_rectangle(>draw, >black, in->data, in->linesize,
> +   in->width, in->height,
> +   s->x + 2 + s->w, s->y - 2, 1, s->h + 5);
> +
> +for (y = 0; y < s->h; y++) {
> +for (x = 0; x < s->w; x++) {
> +FFDrawColor color = { { 0 } };
> +int value[4] = { 0 };
> +
> +s->pick_color(>draw, , in, x + s->x, y + s->y, value);
> +ff_fill_rectangle(>draw, , in->data, in->linesize,
> +  x * w + (210 - (s->w * w)) / 2 + X, y * h + 2 
> + Y, w, h);
> +for (i = 0; i < 4; i++) {
> +average[i] += value[i];
> +min[i]  = FFMIN(min[i], value[i]);
> +max[i]  = FFMAX(max[i], value[i]);
> +}
> +}
> +}
> +
> +for (i = 0; i < 4; i++) {
> +average[i] /= s->w * s->h;
> +}
> +
> +snprintf(text, sizeof(text), "CH  AVG MINMAX\n");
> +draw_text(>draw, in, >white, X + 18, Y + 
> 30 + 200 +  2, text, 0);
> +snprintf(text, sizeof(text), "%c  %07.1f %05d %05d\n", s->is_rgb ? 
> rgba[0] : yuva[0], average[s->rgba_map[0]], min[s->rgba_map[0]], 
> max[s->rgba_map[0]]);
> +draw_text(>draw, in, s->is_rgb ? >red   : >white, X + 18, Y + 
> 30 + 200 + 15, text, 0);
> +snprintf(text, sizeof(text), "%c  %07.1f %05d %05d\n", s->is_rgb ? 
> rgba[1] : yuva[1], average[s->rgba_map[1]], min[s->rgba_map[1]], 
> max[s->rgba_map[1]]);
> +draw_text(>draw, in, s->is_rgb ? >green : >blue,  X + 18, Y + 
> 30 + 200 + 30, text, 0);
> +snprintf(text, sizeof(text), "%c  %07.1f %05d %05d\n", s->is_rgb ? 
> rgba[2] : yuva[2], average[s->rgba_map[2]], min[s->rgba_map[2]], 
> max[s->rgba_map[2]]);
> +draw_text(>draw, in, s->is_rgb ? >blue  : >red,   X + 18, Y + 
> 30 + 200 + 45, text, 0);
> +
> +return ff_filter_frame(outlink, in);
> +}
> +
> +static const AVFilterPad pixscope_inputs[] = {
> +{
> +.name   = "default",
> +.type   = AVMEDIA_TYPE_VIDEO,
> +.filter_frame   = pixscope_filter_frame,
> +.config_props   = pixscope_config_input,
> +.needs_writable = 1,
> +},
> +{ NULL }
> +};
> +
> +static const AVFilterPad pixscope_outputs[] = {
> +{
> +.name = "default",
> +.type = AVMEDIA_TYPE_VIDEO,
> +},
> +{ NULL }
> +};
> +
> +AVFilter ff_vf_pixscope = {
> +.name  = "pixscope",
> +.description   = NULL_IF_CONFIG_SMALL("Pixel data analysis."),
> +.priv_size = sizeof(PixscopeContext),
> +.priv_class= _class,
> +.query_formats = query_formats,
> +.inputs= pixscope_inputs,
> +.outputs   = pixscope_outputs,
> +};
> -- 
> 2.9.3

I tested this filter and find it very useful, but could it be adjusted to 
handle full range video. Currently when I test with a sample such as:

./ffplay -f lavfi -i 
color=c=white:s=640x480,format=yuv444p,lutyuv=y=255:u=248:v=10,pixscope

The outputs list Y=235, U=240, V=16.

Dave Rice

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

Re: [FFmpeg-devel] [PATCHv3] add signature filter for MPEG7 video signature

[Dave Rice]

> On Mar 20, 2017, at 10:13 AM, Paul B Mahol <one...@gmail.com> wrote:
> 
> On 3/20/17, Dave Rice <d...@dericed.com> wrote:
>> On Jan 6, 2017, at 1:34 PM, Gerion Entrup <gerion.entrup.ff...@flump.de>
>> wrote:
>>> 
>>> On Donnerstag, 5. Januar 2017 02:26:23 CET Michael Niedermayer wrote:
>>>> On Wed, Jan 04, 2017 at 05:05:41PM +0100, Gerion Entrup wrote:
>>>>> On Dienstag, 3. Januar 2017 16:58:32 CET Moritz Barsnick wrote:
>>>>>>>> The English opposite of "fine" is "coarse", not "course". :)
>>>>>>> Oops.
>>>>>> 
>>>>>> You still have a few "courses". (The actual variables, not the types. I
>>>>>> don't care *too* much, but might be better for consistency.)
>>>>> You're right. Fixed version attached.
>>>>> 
>>>>> 
>>>>>> From my side - mostly style and docs - it looks fine. Technically, I
>>>>>> can't judge too much. You went through a long review cycle already, so
>>>>>> I assume it's fine for those previous reviewers. They have the last
>>>>>> call anyway.
>>>>> 
>>>>> What about FATE? I'm willing to write a test, but don't know the best
>>>>> way.
>>>>> There are official conditions, whether the signature is standard
>>>>> compliant. I've
>>>>> written a python script to proof that (see previous emails).
>>>>> Nevertheless the
>>>>> checks take relatively long and need 3GB inputvideo, so I assume this is
>>>>> not
>>>>> usable for FATE.
>>>> 
>>>> yes, a 3gb reference is not ok for fate
>>>> 
>>>> 
>>>>> 
>>>>> One way would be, to take a short input video, take the calculated
>>>>> signature
>>>>> and use this as reference, but the standard allow some bitflips and my
>>>>> code
>>>>> has some of them in comparison to the reference signatures.
>>>> 
>>>> then the fate test could/should compare and pass if its within what
>>>> we expect of our code. (which may be stricter than the reference
>>>> allowance)
>>>> there are already tests that do similar for comparing PCM/RAW
>>> Ok, will try to create a test the next days.
>>> 
>>> 
>>>>> +#define OFFSET(x) offsetof(SignatureContext, x)
>>>> 
>>>>> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM
>>>> 
>>>> should contin also  AV_OPT_FLAG_FILTERING_PARAM
>>> Done.
>>> 
>>> 
>>>>> +static int export(AVFilterContext *ctx, StreamContext *sc, int input)
>>>>> +{
>>>>> +SignatureContext* sic = ctx->priv;
>>>>> +char filename[1024];
>>>>> +
>>>>> +if (sic->nb_inputs > 1) {
>>>> 
>>>>> +/* error already handled */
>>>>> +av_get_frame_filename(filename, sizeof(filename),
>>>>> sic->filename, input);
>>>> 
>>>> its more robust to use a av_assert*() on the return code if its assumed
>>>> to be never failing than just a comment as a comment cannot be
>>>> automatcially checked for validity currently.
>>> I chose av_assert0 because the check is done only nb_inputs times.
>>> 
>>> The attached patch also fixes the file writing for every frame the one
>>> input is
>>> longer than the other.
>> 
>> Just bumping this thread. I've been using the patch and find it very helpful
>> and would like to see it in libavfilter.
> 
> I do not care as long its GPL.

Gerion's last post on this thread appears to resolve all review comments but 
indicated that he would create a FATE test for the filter. Since the patch has 
been reviewed, I suggest that the missing FATE test could be a later patch and 
not block consideration of merging the signature filter. As noted, it is 
written with GPL.
Dave Rice

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

Re: [FFmpeg-devel] [PATCHv3] add signature filter for MPEG7 video signature

[Dave Rice]

On Jan 6, 2017, at 1:34 PM, Gerion Entrup <gerion.entrup.ff...@flump.de> wrote:
> 
> On Donnerstag, 5. Januar 2017 02:26:23 CET Michael Niedermayer wrote:
>> On Wed, Jan 04, 2017 at 05:05:41PM +0100, Gerion Entrup wrote:
>>> On Dienstag, 3. Januar 2017 16:58:32 CET Moritz Barsnick wrote:
>>>>>> The English opposite of "fine" is "coarse", not "course". :)
>>>>> Oops.
>>>> 
>>>> You still have a few "courses". (The actual variables, not the types. I
>>>> don't care *too* much, but might be better for consistency.)
>>> You're right. Fixed version attached.
>>> 
>>> 
>>>> From my side - mostly style and docs - it looks fine. Technically, I
>>>> can't judge too much. You went through a long review cycle already, so
>>>> I assume it's fine for those previous reviewers. They have the last
>>>> call anyway.
>>> 
>>> What about FATE? I'm willing to write a test, but don't know the best way. 
>>> There are official conditions, whether the signature is standard compliant. 
>>> I've 
>>> written a python script to proof that (see previous emails). Nevertheless 
>>> the 
>>> checks take relatively long and need 3GB inputvideo, so I assume this is 
>>> not 
>>> usable for FATE.
>> 
>> yes, a 3gb reference is not ok for fate
>> 
>> 
>>> 
>>> One way would be, to take a short input video, take the calculated 
>>> signature 
>>> and use this as reference, but the standard allow some bitflips and my code
>>> has some of them in comparison to the reference signatures.
>> 
>> then the fate test could/should compare and pass if its within what
>> we expect of our code. (which may be stricter than the reference
>> allowance)
>> there are already tests that do similar for comparing PCM/RAW
> Ok, will try to create a test the next days.
> 
> 
>>> +#define OFFSET(x) offsetof(SignatureContext, x)
>> 
>>> +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM
>> 
>> should contin also  AV_OPT_FLAG_FILTERING_PARAM
> Done.
> 
> 
>>> +static int export(AVFilterContext *ctx, StreamContext *sc, int input)
>>> +{
>>> +SignatureContext* sic = ctx->priv;
>>> +char filename[1024];
>>> +
>>> +if (sic->nb_inputs > 1) {
>> 
>>> +/* error already handled */
>>> +av_get_frame_filename(filename, sizeof(filename), sic->filename, 
>>> input);
>> 
>> its more robust to use a av_assert*() on the return code if its assumed
>> to be never failing than just a comment as a comment cannot be
>> automatcially checked for validity currently.
> I chose av_assert0 because the check is done only nb_inputs times.
> 
> The attached patch also fixes the file writing for every frame the one input 
> is
> longer than the other.

Just bumping this thread. I've been using the patch and find it very helpful 
and would like to see it in libavfilter.
Dave Rice

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

Re: [FFmpeg-devel] [PATCH]lavf/matroska: Support codec id V_FFV1 for FFV1.

[Dave Rice]

> On Mar 2, 2017, at 9:59 AM, James Almer <jamr...@gmail.com> wrote:
> 
> On 3/2/2017 11:47 AM, Tobias Rapp wrote:
>> On 02.03.2017 15:22, James Almer wrote:
>>> On 3/2/2017 6:28 AM, Nicolas George wrote:
>>>> Le primidi 11 ventôse, an CCXXV, James Almer a écrit :
>>>>> Ah, i see there's generic code to read and write CodecPrivate elements
>>>>> to and from raw extradata for native codecids where no special handling
>>>>> is required.
>>>>> 
>>>>> In any case, the spec says
>>>>> 
>>>>> "For FFV1 versions 2 or less, Private Data SHOULD NOT be written."
>>>>> 
>>>>> The ffv1 encoder creates extradata for v2 and newer, so the muxer
>>>>> should have a custom case for ffv1 in order to detect v2 streams and
>>>>> avoid writing said extradata.
>>>> 
>>>> I have not looked myself at the situation, I only read what you wrote
>>>> her. According to it, it seems here the FFV1 encoder is violating the
>>>> specification and needs to be fixed.
>>> 
>>> I can't say if the encoder exporting extradata for version 2 is right or
>>> wrong, as muxers or bsfs could still use it for whatever reason, but at
>>> least according to the draft ffv1 spec by Michael, it's to be stored at
>>> the container level *only* on versions 3 and above.
>>> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.1
>> 
>> The IETF draft explicitly excludes to specify FFV1 version 2:
>> https://tools.ietf.org/html/draft-niedermayer-cellar-ffv1-01#section-4.8.1
>> 
>> IIRC there have been some edits to remove references to version 2 in the 
>> IETF draft. In the older document at 
>> http://www.ffmpeg.org/%7Emichael/ffv1.html#configuration-record the section 
>> contains "version >= 2".
>> 
>> Regards,
>> Tobias
> 
> Well, that makes things a bit more complex.
> Maybe the Matroska spec should follow and also avoid mentioning v2?
> That way we could just pretend those files don't exist and not bother
> trying to drop their extradata during muxing.

I agree. In the FFV1 spec, it is careful to say that it intentionally does not 
describe FFV1 version 2. I will submit a patch to the Matroska specification to 
avoid using relative language based on version 2.
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] ffprobe: report field order for video streams

[Dave Rice]

> On Oct 20, 2016, at 3:28 AM, Tobias Rapp <t.r...@noa-archive.com> wrote:
> 
> On 19.10.2016 23:15, Michael Niedermayer wrote:
>> On Wed, Oct 19, 2016 at 02:35:21PM +0200, Tobias Rapp wrote:
>>> From: Rodger Combs <rodger.co...@gmail.com>
>>> 
>>> Reviewed-by: Tobias Rapp <t.r...@noa-archive.com>
>>> ---
>>> doc/ffprobe.xsd |  1 +
>>> ffprobe.c   | 13 +
>>> tests/ref/fate/concat-demuxer-extended-lavf-mxf |  2 +-
>>> tests/ref/fate/concat-demuxer-extended-lavf-mxf_d10 |  2 +-
>>> tests/ref/fate/concat-demuxer-simple1-lavf-mxf  |  2 +-
>>> tests/ref/fate/concat-demuxer-simple1-lavf-mxf_d10  |  2 +-
>>> tests/ref/fate/concat-demuxer-simple2-lavf-ts   |  2 +-
>>> tests/ref/fate/ffprobe_compact  |  4 ++--
>>> tests/ref/fate/ffprobe_csv  |  4 ++--
>>> tests/ref/fate/ffprobe_default  |  2 ++
>>> tests/ref/fate/ffprobe_flat |  2 ++
>>> tests/ref/fate/ffprobe_ini  |  2 ++
>>> 12 files changed, 29 insertions(+), 9 deletions(-)
>> 
>> This too doesnt apply:
>> 
>> Applying: ffprobe: report field order for video streams
>> Using index info to reconstruct a base tree...
>> error: patch failed: tests/ref/fate/ffprobe_compact:27
>> error: tests/ref/fate/ffprobe_compact: patch does not apply
>> error: Did you hand edit your patch?
>> It does not apply to blobs recorded in its index.
>> 
> 
> Could be due to encoding problems or long lines. Have rebased the patch onto 
> current git HEAD and attached it as a file.

I tested this and the new version applied for me. The output of xml=q=1 
validated against the revision. LGTM. Having this data in ffprobe is really 
helpful.
Dave Rice

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

Re: [FFmpeg-devel] [PATCH] avformat/matroskaenc: write DisplayWidth and DisplayHeight elements only if they differ from PixelWidth and PixelHeight

[Dave Rice]

Hi James,

> On Oct 18, 2016, at 11:20 PM, James Almer <jamr...@gmail.com> wrote:
> 
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavformat/matroskaenc.c | 6 --
> tests/fate/matroska.mak   | 2 +-
> 2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 6f2957c..03d5326 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1192,8 +1192,10 @@ static int mkv_write_track(AVFormatContext *s, 
> MatroskaMuxContext *mkv,
> av_log(s, AV_LOG_ERROR, "Overflow in display width\n");
> return AVERROR(EINVAL);
> }
> -put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / 
> display_width_div);
> -put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / 
> display_height_div);
> +if (d_width != par->width || display_width_div != 1 || 
> display_height_div != 1) {
> +put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , d_width / 
> display_width_div);
> +put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, 
> par->height / display_height_div);
> +}
> } else if (display_width_div != 1 || display_height_div != 1) {
> put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYWIDTH , par->width / 
> display_width_div);
> put_ebml_uint(pb, MATROSKA_ID_VIDEODISPLAYHEIGHT, par->height / 
> display_height_div);

You could also make the storage simpler by considering whether or not to write 
DisplayWidth and DisplayHeight independently. For instance:
PixelWidth=720
PixelHeight=480
DisplayWidth=640
DisplayHeight=480

could be simplified to this (as DisplayHeight if unstored, defaults to 
PixelHeight)
PixelWidth=720
PixelHeight=480
DisplayWidth=640

However I'm worried that this may break things when #4489 and/or #5301 are 
resolved. As the DisplayWidth/Height are intended to be used after cropping. In 
the attached example on #5301 it has:

PixelWidth=320
PixelHeight=240
PixelCropBottom=40
PixelCropTop=0
PixelCropLeft=240
PixelCropRight=40
DisplayWidth=320
DisplayHeight=240

If this was remuxed with your patch AND a potential patch to pass PixelCrop 
elements from source mkv to output mkv, then the result would be:

PixelWidth=320
PixelHeight=240
PixelCropBottom=40
PixelCropTop=0
PixelCropLeft=240
PixelCropRight=40

and since the Display Elements are not stored, they would be evaluated as 
DisplayWidth as "PixelWidth - PixelCropLeft - PixelCropRight"
DisplayHeight = "PixelHeight - PixelCropTop - PixelCropBottom"
which is 
DisplayWidth=40
DisplayHeight=200
which is wrong, since the original file intended to show the cropped 40x200 
image in a 320x240 display.

Dave Rice

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/matroskaenc: write a DisplayUnit element when aspect ratio is unknown

[Dave Rice]

> On Oct 15, 2016, at 7:54 PM, Kieran O Leary  wrote:
> 
> Hi,
> 
> On Sat, Oct 15, 2016 at 11:09 PM, James Almer  wrote:
> 
>> 
>> -#sar 0: 1/1
>> +#sar 0: 0/1
>> 
> 
> This commit looks very useful. A few archivists (including myself)
> encountered this issue, so this is a wonderful contribution! Thanks James!

+1. Sometimes it makes sense to preserve uncertainty.
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mov: add option to ignore moov atoms which are detected in free atoms.

[Dave Rice]

> On Oct 13, 2016, at 1:56 PM, Hendrik Leppkes  wrote:
> 
> Am 13.10.2016 19:03 schrieb "liangsi" :
>> 
>> ---
>> libavformat/isom.h | 1 +
>> libavformat/mov.c  | 5 -
>> 2 files changed, 5 insertions(+), 1 deletion(-)
>> 
> 
> Can you elaborate on the reasons of why someone would want this? Preferably
> in the commit message for future consideration.

Shouldn't using the moov atom that is not detected within a free atom be the 
default? Can you elaborate on the reasons of why someone wouldn't want this?
Dave
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/matroska: fix MatroskaVideoFieldOrder enum values

[Dave Rice]

> On Oct 12, 2016, at 3:54 PM, James Almer <jamr...@gmail.com> wrote:
> 
> The spec says
> 
> 9: Interlaced with bottom field displayed first and top field stored first
> 14: Interlaced with top field displayed first and bottom field stored first
> 
> And avcodec.h states
> 
> AV_FIELD_TB,  //< Top coded first, bottom displayed first
> AV_FIELD_BT,  //< Bottom coded first, top displayed first
> 
> Signed-off-by: James Almer <jamr...@gmail.com>
> ---
> libavformat/matroska.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
> index 1ff76e6..15e401c 100644
> --- a/libavformat/matroska.h
> +++ b/libavformat/matroska.h
> @@ -286,8 +286,8 @@ typedef enum {
> MATROSKA_VIDEO_FIELDORDER_UNDETERMINED = 2,
> MATROSKA_VIDEO_FIELDORDER_TT   = 1,
> MATROSKA_VIDEO_FIELDORDER_BB   = 6,
> -MATROSKA_VIDEO_FIELDORDER_BT   = 9,
> -MATROSKA_VIDEO_FIELDORDER_TB   = 14,
> +MATROSKA_VIDEO_FIELDORDER_TB   = 9,
> +MATROSKA_VIDEO_FIELDORDER_BT   = 14,
> } MatroskaVideoFieldOrder;
> 
> typedef enum {
> -- 
> 2.9.1

LGTM. Thanks for fixing this. It now aligns to the same list in QuickTime as 
was intended in the spec.

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/matroskaenc: fix Tags master on seekable output if there are tags after the last stream duration

[Dave Rice]

> On Oct 7, 2016, at 10:30 AM, James Almer <jamr...@gmail.com> wrote:
> 
> On 10/7/2016 3:05 AM, James Almer wrote:
>> The dynamic AVIOContext would get closed pointing to the wrong position
>> in the buffer.
>> This is a regression since 650e17d88b63b5aca6e0a43483e89e64b0f7d2dd.
>> 
>> Signed-off-by: James Almer <jamr...@gmail.com>
>> ---
>> Example:
>> ./ffmpeg -i fate-samples/vorbis/vorbis_chapter_extension_demo.ogg -c:a copy 
>> -metadata:c key=value out.mka
>> 
>> The output is corrupt before this patch.
>> 
>> The refactoring isn't necessary per se, but it comes in handy for the next 
>> patch,
>> where attachments will not be treated as tracks anymore when writting tags.
>> 
>> libavformat/matroskaenc.c | 15 +--
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 0878cb5..286b8b4 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -2264,17 +2264,19 @@ static int mkv_write_trailer(AVFormatContext *s)
>> end_ebml_master_crc32(pb, >info_bc, mkv, mkv->info);
>> 
>> // update stream durations
>> -if (mkv->stream_durations) {
>> +if (!mkv->is_live && mkv->stream_durations) {
>> int i;
>> +int64_t curr = avio_tell(mkv->tags_bc);
>> for (i = 0; i < s->nb_streams; ++i) {
>> AVStream *st = s->streams[i];
>> -double duration_sec = mkv->stream_durations[i] * 
>> av_q2d(st->time_base);
>> -char duration_string[20] = "";
>> 
>> -av_log(s, AV_LOG_DEBUG, "stream %d end duration = %" PRIu64 
>> "\n", i,
>> -   mkv->stream_durations[i]);
>> +if (mkv->stream_duration_offsets[i] > 0) {
>> +double duration_sec = mkv->stream_durations[i] * 
>> av_q2d(st->time_base);
>> +char duration_string[20] = "";
>> +
>> +av_log(s, AV_LOG_DEBUG, "stream %d end duration = %" 
>> PRIu64 "\n", i,
>> +   mkv->stream_durations[i]);
>> 
>> -if (!mkv->is_live && mkv->stream_duration_offsets[i] > 0) {
>> avio_seek(mkv->tags_bc, mkv->stream_duration_offsets[i], 
>> SEEK_SET);
>> 
>> snprintf(duration_string, 20, "%02d:%02d:%012.9f",
>> @@ -2284,6 +2286,7 @@ static int mkv_write_trailer(AVFormatContext *s)
>> put_ebml_binary(mkv->tags_bc, MATROSKA_ID_TAGSTRING, 
>> duration_string, 20);
>> }
>> }
>> +avio_seek(mkv->tags_bc, curr, SEEK_SET);
>> }
>> if (mkv->tags.pos && !mkv->is_live) {
>> avio_seek(pb, mkv->tags.pos, SEEK_SET);
> 
> 
> Ping. It's a recent regression on a muxer, so I'd like to solve it asap.
> I'll push it later today if nobody comments.

I tested this and it does fix the issue so that the added "key" tag is properly 
formed, whereas the tag is only null bytes without the patch. Thanks!
Dave Rice


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

Re: [FFmpeg-devel] [PATCH 00/11] CRC32 support for Matroska muxer

[Dave Rice]

> On Oct 6, 2016, at 4:33 PM, James Almer  wrote:
> 
> On 10/6/2016 3:28 PM, Michael Niedermayer wrote:
>> On Thu, Oct 06, 2016 at 11:27:08AM -0300, James Almer wrote:
>>> On 10/6/2016 7:29 AM, Michael Niedermayer wrote:
 On Mon, Oct 03, 2016 at 08:36:56PM -0300, James Almer wrote:
> This patchsets implements the feature requested on ticket #4347.
> The first three patches are preparation work. The first one isn't
> strictly related to the implementation, but comes in handy
> nonetheless.
> 
> Patches 4 to 11 can be squashed into a single commit before pushing
> if that's prefered, but for review purposes i split things as one
> patch per Level 1 element being adapted to write a CRC32 element.
> 
> Fate tests are updated as needed.
 
 some questions
 Does this reduce writing speed ? for example when remuxing high
 bitrate data like rawvideo ?
>>> 
>>> The bulk of the CRC calculation is done once per level 1 element, so it
>>> shouldn't affect remuxing speed in a considerable way. But if preferred,
>>> i could see about adding an option to disable it.
>> 
>> i guess such a option cant hurt
> 
> Will add it.

Worth adding to the Changelog as well.

>> thanks
>> 
>> no furter comments from me about this patchset
> 
> Pushed without squashing, so if problems arise bisect can more accurately
> point where things went wrong.
> 
> Thanks.
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/3] avformat/matroska: write FlagInterlaced element in WebM

[Dave Rice]

Hi,

> On Oct 2, 2016, at 7:14 PM, James Almer <jamr...@gmail.com> wrote:
> 
> On 9/27/2016 3:03 PM, James Almer wrote:
>> It's listed as supported in both https://www.webmproject.org/docs/container/
>> and https://matroska.org/technical/specs/index.html
>> 
>> Signed-off-by: James Almer <jamr...@gmail.com>
>> ---
>> libavformat/matroskaenc.c | 41 +
>> 1 file changed, 21 insertions(+), 20 deletions(-)
> 
> Ping.

Untested but from a read through, the patch looks good. FlagInterlaced is 
supported in both Matroska and webM, whereas FieldOrder (a new field from the 
CELLAR work on Matroska) is only supported in Matroska. IMHO though I think 
that FlagInterlaced without information on FieldOrder is not so useful and that 
the webM project should consider adopting FieldOrder as well. I'd prefer to see 
a patch to webM to consider adding FieldOrder to the format rather than see 
FieldOrder removed from the webM muxer in FFmpeg.

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

Re: [FFmpeg-devel] [PATCH]lavf/movenc: Allow to disable writing the timecode track

[Dave Rice]

> On Sep 25, 2016, at 1:49 PM, Michael Niedermayer <mich...@niedermayer.cc> 
> wrote:
> 
> On Sat, Sep 24, 2016 at 10:40:35PM +0200, Carl Eugen Hoyos wrote:
>> Hi!
>> 
>> Not everybody is happy about a timecode track in mp4, 
>> see ticket #5492.
>> 
>> Please comment, Carl Eugen
> 
>> doc/muxers.texi  |3 +++
>> libavformat/movenc.c |6 --
>> libavformat/movenc.h |1 +
>> 3 files changed, 8 insertions(+), 2 deletions(-)
>> 1ff491854d9a8c694b38153561d873b55688bb13  
>> 0001-lavf-movenc-Allow-to-disable-writing-the-timecode-tr.patch
>> From 47a813709643106b0d0a2eceff822107c395d15c Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos <ceho...@ag.or.at>
>> Date: Sat, 24 Sep 2016 22:38:14 +0200
>> Subject: [PATCH] lavf/movenc: Allow to disable writing the timecode track.
>> 
>> Fixes ticket #5492.
>> ---
>> doc/muxers.texi  |3 +++
>> libavformat/movenc.c |6 --
>> libavformat/movenc.h |1 +
>> 3 files changed, 8 insertions(+), 2 deletions(-)
> 
> should be ok, but please wait a day with applying so others can
> comment too

+1 Tested this and it works well and solves an issue that I occasionally find. 
Thanks Carl.

[...]

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

Re: [FFmpeg-devel] calculation of probe score

[Dave Rice]

Hi Carl,

> On Sep 13, 2016, at 2:57 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
> 
> 2016-09-12 19:47 GMT+02:00 Monique Lassere <lasse...@gmail.com>:
>> Trying to dig as deeply as possible into ffprobe's probe score
>> and how it is calculated.
> 
> Is there a use case that could be interesting to us?

I think the use case if for when an archive accepts a large diverse collection 
for preservation to access probe_score per file. A lower probe_score may 
indicate that the file is broken, misnamed, mis-categorized, which could mean 
that creation of derivatives or long-term handling may need closer review.

In a related conversation about probe score I tested 2000+ random files and 
made this histogram: rate of occurrence, format name and probe_score.

149 "matroska,webm",100
672 "mov,mp4,m4a,3gp,3g2,mj2",100
  1 aac,1
  1 aac,25
  1 adp,25
 15 aiff,100
  4 asf,100
  8 avi,100
919 caf,100
 45 dv,75
  2 ea,100
  1 flac,1
  1 flac,13
  4 flac,50
  8 flv,100
  3 mp3,25
 90 mp3,51
  9 mpeg,26
  8 mpeg,52
  1 mpegts,50
  1 mpegvideo,12
 13 mpegvideo,51
 11 mxf,100
  1 ogg,100
  2 rm,100
  1 swf,100
  5 swf,26
 92 wav,99

So for instance perhaps worth a closer look at the mpegvideo with score of 12 
or the flac file with score of 1(!), before moving the file into long term 
storage.

> Or a bug you want to tell us about?
> 
> [...]
> 
>> 2. Secondly, does the probe score relate to the container format only?
> 
> As Michael said, yes.
> Note that some containers (like H.264 Annex B or ADTS) correspond
> to a codec, so for containers that "allow" random codecs (like mpeg
> streams) the same probe functions that allow detecting formats can
> be used to detect codecs.

[...]

Best Regards,
Dave Rice

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

Re: [FFmpeg-devel] [VDD2016] Technical Discussion Results

[Dave Rice]

Hi,

> On Sep 12, 2016, at 5:58 AM, Thilo Borgmann <thilo.borgm...@mail.de> wrote:
> 
> Hi,
> 
> these are the resulting notes we've taken during the FFmpeg technical 
> discussion
> during VDD 2016. We've ended up with two slides on the board and compn took
> pictures of them. You can find them here:
> 
> https://drive.google.com/drive/folders/0B53tgCAS2uUnVUpZN0U1WDE0MzQ?usp=sharing
> 
> Additional info:
> 
> - There were two more job offers compn forgotto write down. One from Alex at
> Comcast to implement a perceptional video filter in lavfi for 4K, along with
> mcdeint and denoise filters. Contact compn to get more information about this.
> Also Kodi is looking for python plugin developers. Compn also thinks that they
> are looking for Android devs, too.
> 
> - The group working on ffv1 specification said they've also opportunities for
> work. However you've to contact the commission members at archivist 
> conferences.
> I think we haven't got a list of these conferences, though.

I'm in the CELLAR working group (working on Matorka, FFV1, FLAC) and am also an 
archivist and can speak to this. The MediaConch project via PREFORMA has some 
funds to sponsor FFV1 and Matroska development during this year.

- Option to add CRCs to Top Level Elements in Matroska when muxing. 
http://trac.ffmpeg.org/ticket/4347
- Support for selecting an Ordered Edition in mkv: 
http://trac.ffmpeg.org/ticket/5698
- Bayer pixel format support in FFV1
- Support for additional color information in FFV1 to better handle lossless 
DPX->FFV1->DPX transfer while preserving characteristics such as if the 
sampling is logarithmic or linear.

Probably others. Feel welcome to suggest a bounty or send questions to myself 
or Jerome Martinez <jer...@mediaarea.net>.

[...]

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

Re: [FFmpeg-devel] [PATCH] ffprobe: add -show_headers_first option

[Dave Rice]

> On Sep 4, 2016, at 1:09 PM, Nicolas George <geo...@nsup.org> wrote:
> 
> Le nonidi 19 fructidor, an CCXXIV, Stefano Sabatini a écrit :
>> This is meant to be used for generating output suitable for the
>> ffprobe_default demuxer.
>> ---
>> ffprobe.c | 30 ++
>> 1 file changed, 26 insertions(+), 4 deletions(-)
> 
> I find the naming a bit clumsy. It says what it says, but what it the next
> step? -show_format_after_frames_but_before_streams?
> 
> I would like to suggest another avenue, a little more work but not by much:
> deprecate all the -show_ options, and replace them by a single
> -show option that takes the sections name in order:
> 
> ffprobe -show packets,streams,format
> 
> would be equivalent to the current "ffprobe -show_frames -show_streams
> -show_format", but:
> 
> ffprobe -show format,streams,packets
> 
> would yield the same result as with -show_headers_first.
> 
> The syntax could later be updated for various extra features:
> 
> ffprobe -show format,streams,packets,rewind,packets+frames=silent=1,streams
> 
> I do not know how it interacts with the -show_entries feature, but I must
> say I never understood how the -show_entries feature works.

I’m not clear as to the reasoning for this, as the ordering of the elements 
doesn’t effect any of the meaing of the xml. However I wanted to note that such 
reordering would create invalid ffprobe xml. ffprobe.xsd defines the elements 
in a `sequence`, so if the order of elements is really arbitrary it should be 
changed to a `choice`.
Dave Rice
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc: omit segment UID for webm

[Dave Rice]

> On Jun 22, 2016, at 10:16 AM, Michael Bradshaw <mjbs...@gmail.com> wrote:
> 
> On Tue, Jun 21, 2016 at 12:22 PM, Michael Niedermayer
> <mich...@niedermayer.cc> wrote:
>> what does "supported" / "unsupported" mean, is this defined somewhere ?
> 
> As far as WebM is concerned, SegmentUID doesn't exist.
> 
> In this situation, writing SegmentUID is Undefined-Ignore or
> Optional-Fail/Forbidden, depending on the file. I concur with Dave's
> analysis, but parsing may fail if SegmentUID is written and the master
> element has an unknown size (which will be the case if ffmpeg is
> writing to an unseakable output, like pipes). This is because master
> elements with unknown size are terminated by the first unrecognized
> would-be-child element[1], which would cause the parser to terminate
> the Info element.

I don’t quite think that the EBML specification agrees with this explanation. 
See:

"When EBML is used in transmission or streaming, data that is not part of an 
EBML Element is permitted to be present within a Master-element if 
`unknownsizeallowed` is enabled within that Master-element's definition. In 
this case, the reader should skip data until a valid Element ID of the same 
level or the next greater level of the Master-element is found. What Element 
IDs are considered valid within a Master-element is identified by the EBML 
Schema for that version of the EBML Document Type. Any data contained with a 
Master-element that is not part of an Element SHOULD be ignored."

In this case a SegmentUID Element (undefined in webm) should be skipped be the 
reader rather than cause the termination of the reader.

The end of an unknown-sized Element is determined by the beginning of the next 
known Element that is not a valid sub-element rather than the first unknown 
Element.

> This would continue to screw up parsing, because now
> there could be other children of Info (like DateUTC) after the
> SegmentUID, and since the Info element has terminated and those aren't
> valid children of Segment, it would cause the Segment element to
> terminate. At that point, the file is pretty broken because now there
> is no Segment element containing the Clusters and other elements.
> 
> [1]: "The end of a Master-element with unknown size is determined by
> the beginning of the next element that is not a valid sub-element of
> that Master-element" (from the current EBML spec)

Best Regards,
Dave Rice

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

Re: [FFmpeg-devel] [PATCH] libavformat/matroskaenc: omit segment UID for webm

[Dave Rice]

> On Jun 21, 2016, at 3:22 PM, Michael Niedermayer <mich...@niedermayer.cc> 
> wrote:
> 
> On Mon, Jun 20, 2016 at 08:20:47PM -0700, Michael Bradshaw wrote:
>> Hi,
>> 
>> Attached patch removes SegmentUID element from WebM files, as it's
>> unsupported in WebM[1].
>> 
>> Please review/apply.
>> 
>> Thanks,
>> 
>> Michael
>> 
>> [1]: https://www.webmproject.org/docs/container/#SegmentUID
> 
> what does "supported" / "unsupported" mean, is this defined somewhere ?
> 
> In a format something can be
> Forbidden, aka
>you write it you are buggy, you see it you can bail out
> 
> Required, aka
>you dont write it you are buggy, you dont see it you can bail out
> 
> Optional-Ignore, aka
>you may write it if you like, you may ignore it if you like, you must not 
> bail out
> 
> Optional-Fail, aka
>you may write it if you like, you may bail out if you see it
> 
> Undefined-Ignore
>you write it you are buggy, you may ignore it if you like, you must not 
> bail out
> 
> 
> The reason why this question matters is because
> If its forbidden to be written then the patch is correct and the
> only option, OTOH
> if its allowed to be written but the demuxer can ignore it there may
> or may not be arguments to write it anyway

[...]

From my reading of the current EBML specification this is "Undefined-Ignore", 
but is also something that should not be done. The closest sentence on this 
from the EBML specification seems to be: "Element Data stored within 
Master-elements SHOULD only consist of EBML Elements and SHOULD NOT contain any 
data that is not part of an EBML Element." I think "EBML Element" in this case 
should be considered to mean "EBML Elements as defined by the EBML DocType". 
The webm EBML Schema does not define a SegmentUID Element as Matroska does, so 
the Element is meaningless in webm. It may happen to follow the EBML Header 
pattern but in the context of webm it is junk data.

Best Regards,
Dave Rice





signature.asc
Description: Message signed with OpenPGP using GPGMail
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] avfilter/vf_histogram: indent histogram options

[Dave Rice]

Some indentation after the previous patch.
Dave Rice


0002-avfilter-vf_histogram-indent-histogram-options.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] avfilter/vf_histogram: shortcuts for histogram options

[Dave Rice]

This patch adds some shortcuts for the histogram filter's options in a way that 
is consistent with the waveform and vectorscope filter.
Dave Rice



0001-avfilter-vf_histogram-shortcuts-for-histogram-option.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/matroskaenc.c: use metadata key/value to set mastering metadata

[Dave Rice]

> On Jun 8, 2016, at 11:03 PM, Neil Birkbeck <neil.birkb...@gmail.com> wrote:
> 
> The most recent patch should still apply cleanly (unless we have a
> better way to set these elements). Thanks

There are specific mastering metadata elements as recently defined in the 
Matroska specification [1]. It would be better to use these defined elements 
from the specification rather than somehow fit this data into tags.
Dave Rice

[1] https://matroska.org/technical/specs/index.html#MasteringMetadata

> On Tue, Apr 19, 2016 at 8:12 AM, Neil Birkbeck <neil.birkb...@gmail.com> 
> wrote:
>> Updated patch attached.
>> 
>> On Sat, Apr 16, 2016 at 7:08 PM, Michael Niedermayer
>> <mich...@niedermayer.cc> wrote:
>>> On Sun, Apr 03, 2016 at 03:38:33PM -0700, Neil Birkbeck wrote:
>>>> Use "master_display" key/value pair to specify mastering metadata in a
>>>> similar formatting as accepted by libx265 (unless there is some other
>>>> generic way to add side data to a stream from command line).
>>>> Currently, the packet side data propagates from an input file to
>>>> output file if it is transmuxed (mkv -> mkv). Perhaps we want to also
>>>> use this same metadata key in matroskadec to also allow for the
>>>> metadata to propagate during transcoding.
>>> 
>>>> matroskaenc.c |   51 +++
>>>> 1 file changed, 47 insertions(+), 4 deletions(-)
>>>> d92564f3ec6cf08430a79b64f4d1ec304637afe1  
>>>> 0001-lavf-matroskaenc.c-use-metadata-key-value-to-set-mas.patch
>>>> From b30d80f6ba4b09811039f64af3e7f709d86df5fe Mon Sep 17 00:00:00 2001
>>>> From: Neil Birkbeck <neil.birkb...@gmail.com>
>>>> Date: Fri, 1 Apr 2016 17:02:42 -0700
>>>> Subject: [PATCH] lavf/matroskaenc.c: use metadata key/value to set 
>>>> mastering
>>>> metadata
>>>> 
>>>> Add key/value metadata interface to allow command line setting of 
>>>> AVMasteringDisplayMetadata. The formatting is the same as the option in 
>>>> libx265.
>>>> 
>>>> Signed-off-by: Neil Birkbeck <neil.birkb...@gmail.com>
>>> 
>>> this does not apply cleanly anymore after codecpar
>>> 
>>> Applying: lavf/matroskaenc.c: use metadata key/value to set mastering 
>>> metadata
>>> Using index info to reconstruct a base tree...
>>> Falling back to patching base and 3-way merge...
>>> Auto-merging libavformat/matroskaenc.c
>>> CONFLICT (content): Merge conflict in libavformat/matroskaenc.c
>>> Failed to merge in the changes.
>>> Patch failed at 0001 lavf/matroskaenc.c: use metadata key/value to set 
>>> mastering metadata
>>> When you have resolved this problem run "git am --resolved".
>>> If you would prefer to skip this patch, instead run "git am --skip".
>>> To restore the original branch and stop patching run "git am --abort".
>>> 
>>> 
>>> [...]
>>> 
>>> --
>>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>> 
>>> Democracy is the form of government in which you can choose your dictator
>>> 
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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