Re: [FFmpeg-devel] [PATCH 5/9] sbc: implement SBC encoder (low-complexity subband codec)
On Thu, Feb 22, 2018 at 06:18:48PM +, Rostislav Pehlivanov wrote:
> On 21 February 2018 at 22:37, Aurelien Jacobs wrote:
>
> > This was originally based on libsbc, and was fully integrated into ffmpeg.
> > ---
> > doc/general.texi | 2 +-
> > libavcodec/Makefile | 1 +
> > libavcodec/allcodecs.c | 1 +
> > libavcodec/sbcdsp.c | 382 ++
> > +
> > libavcodec/sbcdsp.h | 83 ++
> > libavcodec/sbcdsp_data.c | 329 +
> > libavcodec/sbcdsp_data.h | 55 +++
> > libavcodec/sbcenc.c | 411 ++
> > +
> > 8 files changed, 1263 insertions(+), 1 deletion(-)
> > create mode 100644 libavcodec/sbcdsp.c
> > create mode 100644 libavcodec/sbcdsp.h
> > create mode 100644 libavcodec/sbcdsp_data.c
> > create mode 100644 libavcodec/sbcdsp_data.h
> > create mode 100644 libavcodec/sbcenc.c
> >
> > +
> > +#define OFFSET(x) offsetof(SBCEncContext, x)
> > +#define AE AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> > +static const AVOption options[] = {
> > +{ "joint_stereo", "use joint stereo",
> > + OFFSET(joint_stereo), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, AE },
> >
> +{ "dual_channel", "use dual channel",
> > + OFFSET(dual_channel), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, AE },
> >
>
> Erm those 2 things should be decided by the encoder, not by exposing them
> to the user. The encoder should decide which mode has lower distortion for
> a given signal.
See bellow.
> > +{ "subbands", "number of subbands (4 or 8)",
> > + OFFSET(subbands), AV_OPT_TYPE_INT, { .i64 = 8 }, 4, 8, AE },
> >
>
> The encoder doesn't check if the value isn't 4 or 8 so 5, 6 and 7 are all
> accepted. Similar issue to the previous option too.
OK, fixed.
> > +{ "bitpool", "bitpool value",
> > + OFFSET(bitpool), AV_OPT_TYPE_INT, { .i64 = 32 }, 0, 255, AE },
> >
>
> This should be controlled by the bitrate setting. Either have a function to
> translate bitrate to bitpool value or a table which approximately maps
> bitrate values supplied to bitpools. You could expose it directly as well
> as mapping it to a bitrate value by using the global_quality setting so it
> shouldn't be a custom encoder option.
Indeed, this is better this way, thanks for the suggestion.
> > +{ "blocks", "number of blocks (4, 8, 12 or 16)",
> > + OFFSET(blocks), AV_OPT_TYPE_INT, { .i64 = 16 }, 4, 16, AE },
> > +{ "snr", "use SNR mode (instead of loudness)",
> > + OFFSET(allocation), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, AE },
> >
>
> SNR mode too needs to be decided by the encoder rather than exposing it as
> a setting.
See bellow.
> > +{ "msbc", "use mSBC mode (wideband speech mono SBC)",
> >
>
> Add a profile fallback setting for this as well, like in aac where -aac_ltp
> turns LTP mode on and -profile:a aac_ltp does the same.
Not sure of the benefits of having 2 redundant way to do this, but OK.
> You don't have to make the encoder decide which stereo coupling mode or
> snr/loudness setting to use, you can implement that with a later patch.
> I think you should remove the "blocks" and "subbands" settings as well and
> instead replace those with a single "latency" setting like the native Opus
> encoder in milliseconds which would adjust both of them on init to set the
> frame size. This would also allow the encoder to change them. Again, you
> don't have to do this now, you can send a patch which adds a "latency"
> option later.
I can see the value in what you propose, and I think that indeed, it
would be great for the encoder to choose by itself the most appropriate
parameters by default.
But on the other hand, we are talking about a codec targetting some
hardware decoders (blutetooth speakers, headsets...), and all those
parameters have to be negotiated between the encoder and the hardware
decoder. So I think it is mandatory to keep all those parameters
accessible to be able to setup the encoder according to the target
hardware capabilities.
> Apart from that, I tested the encoder, valgrind looks clean, the SIMD is
> bitexact and all advertised samplerates are supported.
Great.
Updated patch attached.>From e0fa5e81606047e31c4f9d0c7773f82a044473f3 Mon Sep 17 00:00:00 2001
From: Aurelien Jacobs
Date: Sun, 17 Dec 2017 19:59:30 +0100
Subject: [PATCH 5/9] sbc: implement SBC encoder (low-complexity subband codec)
This was originally based on libsbc, and was fully integrated into ffmpeg.
---
doc/general.texi | 2 +-
libavcodec/Makefile| 1 +
libavcodec/allcodecs.c | 1 +
libavcodec/avcodec.h | 2 +
libavcodec/options_table.h | 1 +
libavcodec/profiles.c | 5 +
libavcodec/profiles.h | 1 +
libavcodec/sbcdsp.c| 382 +++
libavcodec/sbcdsp.h| 83 +
libavcodec/sbcd
Re: [FFmpeg-devel] [PATCH 7/9] sbcenc: add MMX optimizations
On Thu, Feb 22, 2018 at 05:21:57PM +, Rostislav Pehlivanov wrote: > On 21 February 2018 at 22:37, Aurelien Jacobs wrote: > [...] > > +;*** > > +;void ff_sbc_analyze_4(const int16_t *in, int32_t *out, const int16_t > > *consts); > > +;*** > > +INIT_MMX mmx > > +cglobal sbc_analyze_4, 3, 3, 4, in, out, consts > > +movq m0, [inq] > > +movq m1, [inq+8] > > +pmaddwd m0, [constsq] > > +pmaddwd m1, [constsq+8] > > +paddd m0, [scale_mask] > > +paddd m1, [scale_mask] > > + > > +movq m2, [inq+16] > > +movq m3, [inq+24] > > +pmaddwd m2, [constsq+16] > > +pmaddwd m3, [constsq+24] > > +paddd m0, m2 > > +paddd m1, m3 > > + > > +movq m2, [inq+32] > > +movq m3, [inq+40] > > +pmaddwd m2, [constsq+32] > > +pmaddwd m3, [constsq+40] > > +paddd m0, m2 > > +paddd m1, m3 > > + > > +movq m2, [inq+48] > > +movq m3, [inq+56] > > +pmaddwd m2, [constsq+48] > > +pmaddwd m3, [constsq+56] > > +paddd m0, m2 > > +paddd m1, m3 > > + > > +movq m2, [inq+64] > > +movq m3, [inq+72] > > +pmaddwd m2, [constsq+64] > > +pmaddwd m3, [constsq+72] > > +paddd m0, m2 > > +paddd m1, m3 > > > > You can macro the top 3 blocks > > [...] > > +;*** > > +;void ff_sbc_analyze_8(const int16_t *in, int32_t *out, const int16_t > > *consts); > > +;*** > > +INIT_MMX mmx > > +cglobal sbc_analyze_8, 3, 3, 4, in, out, consts > > +movq m0, [inq] > > +movq m1, [inq+8] > > +movq m2, [inq+16] > > +movq m3, [inq+24] > > +pmaddwd m0, [constsq] > > +pmaddwd m1, [constsq+8] > > +pmaddwd m2, [constsq+16] > > +pmaddwd m3, [constsq+24] > > +paddd m0, [scale_mask] > > +paddd m1, [scale_mask] > > +paddd m2, [scale_mask] > > +paddd m3, [scale_mask] > > + > > +movq m4, [inq+32] > > +movq m5, [inq+40] > > +movq m6, [inq+48] > > +movq m7, [inq+56] > > +pmaddwd m4, [constsq+32] > > +pmaddwd m5, [constsq+40] > > +pmaddwd m6, [constsq+48] > > +pmaddwd m7, [constsq+56] > > +paddd m0, m4 > > +paddd m1, m5 > > +paddd m2, m6 > > +paddd m3, m7 > > + > > +movq m4, [inq+64] > > +movq m5, [inq+72] > > +movq m6, [inq+80] > > +movq m7, [inq+88] > > +pmaddwd m4, [constsq+64] > > +pmaddwd m5, [constsq+72] > > +pmaddwd m6, [constsq+80] > > +pmaddwd m7, [constsq+88] > > +paddd m0, m4 > > +paddd m1, m5 > > +paddd m2, m6 > > +paddd m3, m7 > > + > > +movq m4, [inq+96] > > +movq m5, [inq+104] > > +movq m6, [inq+112] > > +movq m7, [inq+120] > > +pmaddwd m4, [constsq+96] > > +pmaddwd m5, [constsq+104] > > +pmaddwd m6, [constsq+112] > > +pmaddwd m7, [constsq+120] > > +paddd m0, m4 > > +paddd m1, m5 > > +paddd m2, m6 > > +paddd m3, m7 > > + > > +movq m4, [inq+128] > > +movq m5, [inq+136] > > +movq m6, [inq+144] > > +movq m7, [inq+152] > > +pmaddwd m4, [constsq+128] > > +pmaddwd m5, [constsq+136] > > +pmaddwd m6, [constsq+144] > > +pmaddwd m7, [constsq+152] > > +paddd m0, m4 > > +paddd m1, m5 > > +paddd m2, m6 > > +paddd m3, m7 > > > > And those 5 blocks > > > > + > > +psrad m0, 16; SBC_PROTO_FIXED_SCALE > > +psrad m1, 16; SBC_PROTO_FIXED_SCALE > > +psrad m2, 16; SBC_PROTO_FIXED_SCALE > > +psrad m3, 16; SBC_PROTO_FIXED_SCALE > > + > > +packssdw m0, m0 > > +packssdw m1, m1 > > +packssdw m2, m2 > > +packssdw m3, m3 > > + > > +movq m4, m0 > > +movq m5, m0 > > +pmaddwd m4, [constsq+160] > > +pmaddwd m5, [constsq+168] > > + > > +movq m6, m1 > > +movq m7, m1 > > +pmaddwd m6, [constsq+192] > > +pmaddwd m7, [constsq+200] > > +paddd m4, m6 > > +paddd m5, m7 > > + > > +movq m6, m2 > > +movq m7, m2 > > +pmaddwd m6, [constsq+224] > > +pmaddwd m7, [constsq+232] > > +paddd
Re: [FFmpeg-devel] [PATCH 2/3] avformat/dashenc: opening a segment file when its first frame is ready
On Mon, 19 Feb 2018, vdi...@akamai.com wrote:
From: Vishwanath Dixit
---
libavformat/dashenc.c | 57 ---
1 file changed, 36 insertions(+), 21 deletions(-)
diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
index 0f6f4f2..0eb4b25 100644
--- a/libavformat/dashenc.c
+++ b/libavformat/dashenc.c
@@ -81,6 +81,9 @@ typedef struct OutputStream {
char bandwidth_str[64];
char codec_str[100];
+char filename[1024];
+char full_path[1024];
+char temp_path[1024];
} OutputStream;
I know it's late, but in the future please work toward supporting
unlimited path lengths, that was the whole point of the deprecation of
AVFormatContext->filename.
Thanks,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] avformat/dashenc: opening a segment file when its first frame is ready
On 2/24/2018 12:19 PM, Marton Balint wrote:
>
>
> On Mon, 19 Feb 2018, vdi...@akamai.com wrote:
>
>> From: Vishwanath Dixit
>>
>> ---
>> libavformat/dashenc.c | 57
>> ---
>> 1 file changed, 36 insertions(+), 21 deletions(-)
>>
>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>> index 0f6f4f2..0eb4b25 100644
>> --- a/libavformat/dashenc.c
>> +++ b/libavformat/dashenc.c
>> @@ -81,6 +81,9 @@ typedef struct OutputStream {
>> char bandwidth_str[64];
>>
>> char codec_str[100];
>> + char filename[1024];
>> + char full_path[1024];
>> + char temp_path[1024];
>> } OutputStream;
>
> I know it's late, but in the future please work toward supporting
> unlimited path lengths, that was the whole point of the deprecation of
> AVFormatContext->filename.
>
> Thanks,
> Marton
It's not late, it can and should be changed. New code using deprecated
APIs that will require changes in the long run should have not been
added, so better change it now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Building FFmpeg dylibs for OSX
Will examine that. Kolya > On Feb 23, 2018, at 11:52 PM, Helmut K. C. Tessarek > wrote: > > On 2018-02-24 01:22, livinginlosange...@mac.com wrote: >> I can successfully compile ffmpeg info lgpl compliant dylibs which I can >> link to from /usr/local. I specify /usr/local during my ./configure phase. >> All OK. I want to be able to include them as dylibs in my OSX app package. >> The app wants to see dylibs which are built using @rpath like >> @rpath/libavutil.dylib when inspecting the dylib using otool. Would anyone >> have any recommendations on how to accomplish this? I must include ffmpeg >> dylibs in my app. > > You can have a look at how the VLC project does it. All of their libs > are dynamic libs which are located in one sub directory of the app. > > So you have to set the rpath relative to your binary that makes the calls. > > Cheers, > K. C. > > -- > regards Helmut K. C. Tessarek KeyID 0x172380A011EF4944 > Key fingerprint = 8A55 70C1 BD85 D34E ADBC 386C 1723 80A0 11EF 4944 > > /* > Thou shalt not follow the NULL pointer for chaos and madness > await thee at its end. > */ > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] Add muxer/demuxer for raw codec2 and .c2 files
ons 2018-02-14 klockan 10:20 +0100 skrev Tomas Härdin: > tis 2018-02-13 klockan 11:48 +0100 skrev Tomas Härdin: > > fre 2018-02-09 klockan 11:29 +0100 skrev Carl Eugen Hoyos: > > > 2018-01-15 22:36 GMT+01:00 Tomas Härdin : > > > > > > > > +if (p->buf[4] > EXPECTED_CODEC2_MINOR_VERSION) score -= > > > > > AVPROBE_SCORE_MAX/5; > > > > > +if (p->buf[5] > AVPRIV_CODEC2_MODE_MAX)score -= > > > > > AVPROBE_SCORE_MAX/5; > > > > > +if (p->buf[6] != 0) score -= > > > > > AVPROBE_SCORE_MAX/5; > > > > > +return score; > > > > > > > > > > Imo, this is too complicated: > > > > > If the first 32bit are correct, return MAX/2, for 24bit, > > > > > return > > > > > less > > > > > > This should have been AVPROBE_SCORE_EXTENSION + 1, > > > sorry about my mistake. > > > > Done. > > > > > Please threaten to push this and push after a few days. > > > > Alright, rebased. I'll push on Sunday if there's no objections > > > > /Tomas > > Aaaand a set that actually passes FATE this time :) Pushed /Tomas signature.asc Description: This is a digitally signed message part ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avformat/aviobuf: add ff_read_line_to_bprint and ff_read_line_to_bprint_overwrite functions
On Wed, 14 Feb 2018, Nicolas George wrote: Marton Balint (2018-02-11): It is a 500% speed improvement on a 260 MB line compared to using av_bprint_chars, so I'd rather leave it as is. I can add a comment saying "for performance reasons we fill a temporary buffer, and use av_bprint functions on chunks of data". This is assuming reading the text file is the only thing that happens. In practice, the lines will be parsed, tokenized, with probably quite a few mallocs, making the overhead negligible. And if performance were really critic, reading the file whole and splitting in memory would probably be best. But of course, since the optimization is already written and gives a significant difference on pure tests, I have no objection as is. Thanks, applied the series. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] fate/exr : add test for ticket 6994 (long name flag)
2018-02-13 12:30 GMT+01:00 Martin Vignali : > > > 2018-02-09 9:22 GMT+01:00 Martin Vignali : > >> Hello, >> >> Patch in attach add test for ticket 6994 (flag long name) >> >> Sample can be found here : >> https://we.tl/WBnt10VSA1 >> >> and need to be put inside ./fate-suite/exr >> >> Can be test with : >> make fate-exr SAMPLES=fate-suite/ >> >> >> Need to be apply after one of the patch in discussion : "lavc/exr: Ignore >> long names flag" >> >> >> >> > Sample upload some days ago Pushed. Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] avfilter/vf/blend : add 16 bits SIMD version for basic mode (v2)
2018-02-17 21:51 GMT+01:00 Martin Vignali : > Hello, > > New patchs in attach > Limit the asm version to x86_64 > > tested on osx X86_64 and osx X86_32 > > > Martin > Pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/exr: Ignore long names flag
> > Hello, > > If no one is against, i will apply the patch in attach in few days (change > since previous patch (mention ticket in commit msg)) > > Martin > > Pushed ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/9] sbc: implement SBC encoder (low-complexity subband codec)
On 24 February 2018 at 12:01, Aurelien Jacobs wrote:
> On Thu, Feb 22, 2018 at 06:18:48PM +, Rostislav Pehlivanov wrote:
> > On 21 February 2018 at 22:37, Aurelien Jacobs wrote:
> >
> > > This was originally based on libsbc, and was fully integrated into
> ffmpeg.
> > > ---
> > > doc/general.texi | 2 +-
> > > libavcodec/Makefile | 1 +
> > > libavcodec/allcodecs.c | 1 +
> > > libavcodec/sbcdsp.c | 382 ++
> > > +
> > > libavcodec/sbcdsp.h | 83 ++
> > > libavcodec/sbcdsp_data.c | 329 +
> > > libavcodec/sbcdsp_data.h | 55 +++
> > > libavcodec/sbcenc.c | 411 ++
> > > +
> > > 8 files changed, 1263 insertions(+), 1 deletion(-)
> > > create mode 100644 libavcodec/sbcdsp.c
> > > create mode 100644 libavcodec/sbcdsp.h
> > > create mode 100644 libavcodec/sbcdsp_data.c
> > > create mode 100644 libavcodec/sbcdsp_data.h
> > > create mode 100644 libavcodec/sbcenc.c
> > >
> > > +
> > > +#define OFFSET(x) offsetof(SBCEncContext, x)
> > > +#define AE AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> > > +static const AVOption options[] = {
> > > +{ "joint_stereo", "use joint stereo",
> > > + OFFSET(joint_stereo), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> AE },
> > >
> > +{ "dual_channel", "use dual channel",
> > > + OFFSET(dual_channel), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> AE },
> > >
> >
> > Erm those 2 things should be decided by the encoder, not by exposing them
> > to the user. The encoder should decide which mode has lower distortion
> for
> > a given signal.
>
> See bellow.
>
> > > +{ "subbands", "number of subbands (4 or 8)",
> > > + OFFSET(subbands), AV_OPT_TYPE_INT, { .i64 = 8 }, 4, 8,
> AE },
> > >
> >
> > The encoder doesn't check if the value isn't 4 or 8 so 5, 6 and 7 are all
> > accepted. Similar issue to the previous option too.
>
> OK, fixed.
>
> > > +{ "bitpool", "bitpool value",
> > > + OFFSET(bitpool), AV_OPT_TYPE_INT, { .i64 = 32 }, 0, 255,
> AE },
> > >
> >
> > This should be controlled by the bitrate setting. Either have a function
> to
> > translate bitrate to bitpool value or a table which approximately maps
> > bitrate values supplied to bitpools. You could expose it directly as well
> > as mapping it to a bitrate value by using the global_quality setting so
> it
> > shouldn't be a custom encoder option.
>
> Indeed, this is better this way, thanks for the suggestion.
>
> > > +{ "blocks", "number of blocks (4, 8, 12 or 16)",
> > > + OFFSET(blocks), AV_OPT_TYPE_INT, { .i64 = 16 }, 4, 16,
> AE },
> > > +{ "snr", "use SNR mode (instead of loudness)",
> > > + OFFSET(allocation), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> AE },
> > >
> >
> > SNR mode too needs to be decided by the encoder rather than exposing it
> as
> > a setting.
>
> See bellow.
>
> > > +{ "msbc", "use mSBC mode (wideband speech mono SBC)",
> > >
> >
> > Add a profile fallback setting for this as well, like in aac where
> -aac_ltp
> > turns LTP mode on and -profile:a aac_ltp does the same.
>
> Not sure of the benefits of having 2 redundant way to do this, but OK.
>
> > You don't have to make the encoder decide which stereo coupling mode or
> > snr/loudness setting to use, you can implement that with a later patch.
> > I think you should remove the "blocks" and "subbands" settings as well
> and
> > instead replace those with a single "latency" setting like the native
> Opus
> > encoder in milliseconds which would adjust both of them on init to set
> the
> > frame size. This would also allow the encoder to change them. Again, you
> > don't have to do this now, you can send a patch which adds a "latency"
> > option later.
>
> I can see the value in what you propose, and I think that indeed, it
> would be great for the encoder to choose by itself the most appropriate
> parameters by default.
> But on the other hand, we are talking about a codec targetting some
> hardware decoders (blutetooth speakers, headsets...), and all those
> parameters have to be negotiated between the encoder and the hardware
> decoder. So I think it is mandatory to keep all those parameters
> accessible to be able to setup the encoder according to the target
> hardware capabilities.
>
>
Hardware isn't as limited as you might think it is, and there's also no
negotiation happening between encoders and decoders IRL (except for modern
streaming protocols which suck). While its true that we've had some issues
with hardware decoders not supporting rarely used features we wait for
users to report them and don't assume that all hardware violates the specs.
And looking at the code, only 2 settings strike out as being able to cause
issues: the frame size, controlled by subbands and blocks and the msbc
mode. There's no way in hell that dual stereo and joint coding
Re: [FFmpeg-devel] Fix memset size on ctts_data in mov_read_trun()
On Fri, Feb 23, 2018 at 05:12:05PM -0800, Xiaohan Wang (王消寒) wrote: > Michael: Dale and I dig into history a bit more and we don't understand why > the code is changed to the current state around memset. This new patch > reverted the change back to the previous state which we felt is much > cleaner. Please see the CL description for details and take a look at the > new patch. Thanks! > > On Wed, Feb 21, 2018 at 1:14 PM, Xiaohan Wang (王消寒) > wrote: > > > jstebbins: kindly ping! > > > > On Fri, Feb 16, 2018 at 2:42 PM, Xiaohan Wang (王消寒) > > wrote: > > > >> +jstebbins@ who wrote that code. > >> > >> On Fri, Feb 16, 2018 at 12:30 PM, Michael Niedermayer < > >> mich...@niedermayer.cc> wrote: > >> > >>> On Thu, Feb 15, 2018 at 12:10:33PM -0800, Xiaohan Wang (王消寒) wrote: > >>> > > >>> > >>> > mov.c |3 ++- > >>> > 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > 5597d0b095f8b15eb11503010a51c2bc2c022413 > >>> 0001-ffmpeg-Fix-memset-size-on-ctts_data-in-mov_read_trun.patch > >>> > From 7c1e6b50ebe35b2a38c4f1d0a988e31eccbd0ead Mon Sep 17 00:00:00 2001 > >>> > From: Xiaohan Wang > >>> > Date: Thu, 15 Feb 2018 12:05:53 -0800 > >>> > Subject: [PATCH] ffmpeg: Fix memset size on ctts_data in > >>> mov_read_trun() > >>> > > >>> > The allocated size of sc->ctts_data is > >>> > (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). > >>> > > >>> > The size to memset at offset sc->ctts_data + sc->ctts_count should be > >>> > (st->nb_index_entries + entries - sc->ctts_count) * > >>> sizeof(*sc->ctts_data)) > >>> > > >>> > The current code missed |entries| I believe. > >>> > >>> shouldnt "entries" be read by this function later and so shouldnt need a > >>> memset? > >>> I didnt write this, but it looks a bit to me as if it was intended to > >>> only > >>> clear the area that would not be read later > >>> > >> > >> I thought we only had sc->ctts_count entries before av_fast_realloc, so > >> memset everything starting from sc->ctts_data + sc->ctts_count couldn't > >> go wrong. But I am not familiar with this code and that could totally be > >> wrong. I added jstebbins@ who wrote the code and hopefully we can get > >> expert opinion there. > >> > >> > >>> [...] > >>> -- > >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > >>> > >>> No great genius has ever existed without some touch of madness. -- > >>> Aristotle > >>> > >>> ___ > >>> ffmpeg-devel mailing list > >>> ffmpeg-devel@ffmpeg.org > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>> > >>> > >> > > > mov.c |5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > e5bbe55f0b1260f787f431b5c45e6ca49a7d2d1e > 0001-Fix-memset-size-on-ctts_data-in-mov_read_trun-round-.patch > From f34b35ec5749c17b21f80665a0b8859bce3e84ab Mon Sep 17 00:00:00 2001 > From: Xiaohan Wang > Date: Fri, 23 Feb 2018 10:51:30 -0800 > Subject: [PATCH] Fix memset size on ctts_data in mov_read_trun() (round 2) > > The allocated size of sc->ctts_data is > (st->nb_index_entries + entries) * sizeof(*sc->ctts_data). > > The size to memset at offset sc->ctts_data + sc->ctts_count should be > (st->nb_index_entries + entries - sc->ctts_count) * > sizeof(*sc->ctts_data)) > > The current code missed |entries| I believe, which was introduced in > https://patchwork.ffmpeg.org/patch/5541/. > > However, after offline discussion, it seems the original code is much > more clear to read (before https://patchwork.ffmpeg.org/patch/5541/). > > Hence this CL revert the memset logic to it's previous state by > remembering the |old_ctts_allocated_size|, and only memset the newly > allocated entries. > --- > libavformat/mov.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index a3725692a7..f8d79c7b02 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -4713,6 +4713,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext > *pb, MOVAtom atom) > st->index_entries= new_entries; > > requested_size = (st->nb_index_entries + entries) * > sizeof(*sc->ctts_data); > +unsigned int old_ctts_allocated_size = sc->ctts_allocated_size; this should possibly be size_t and declaration and statements should not be mixed libavformat/mov.c: In function ‘mov_read_trun’: libavformat/mov.c:4691:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you fake or manipulate statistics in a paper in physics you will never get a job again. If you fake or manipulate statistics in a paper in medicin you will get a job for life at the pharma industry. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] avformat/dashenc: opening a segment file when its first frame is ready
On 2/24/18 9:11 PM, James Almer wrote:
> On 2/24/2018 12:19 PM, Marton Balint wrote:
>>
>>
>> On Mon, 19 Feb 2018, vdi...@akamai.com wrote:
>>
>>> From: Vishwanath Dixit
>>>
>>> ---
>>> libavformat/dashenc.c | 57
>>> ---
>>> 1 file changed, 36 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
>>> index 0f6f4f2..0eb4b25 100644
>>> --- a/libavformat/dashenc.c
>>> +++ b/libavformat/dashenc.c
>>> @@ -81,6 +81,9 @@ typedef struct OutputStream {
>>> char bandwidth_str[64];
>>>
>>> char codec_str[100];
>>> +char filename[1024];
>>> +char full_path[1024];
>>> +char temp_path[1024];
>>> } OutputStream;
>>
>> I know it's late, but in the future please work toward supporting
>> unlimited path lengths, that was the whole point of the deprecation of
>> AVFormatContext->filename.
Thanks for pointing it out. Sure, we will fix it. We had done these long
back(atleast 6 months back), when filename was still in active use.
Only now we had the time to merge these changes with the latest ffmpeg.
>>
>> Thanks,
>> Marton
>
> It's not late, it can and should be changed. New code using deprecated
> APIs that will require changes in the long run should have not been
> added, so better change it now.
Just to clarify here, we are not using any deprecated API in that patch. We are
still using the url parameter.
In fact, yesterday I sent out a different patch to fix deprecated API usage
that was previously present.
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225739.html
The only issue was that the size of filename has been limited to 1024
internally. This needs to be replaced by a dynamic size buffer.
But I agree with you that it is better to change now rather than later.
> ___
> 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
