Re: [FFmpeg-devel] [PATCH] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On Tue, Jul 02, 2019 at 08:42:43PM -0300, James Almer wrote: > On 7/2/2019 5:56 PM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer > > --- > > doc/APIchanges | 3 +++ > > libavutil/mem.h | 13 + > > libavutil/version.h | 2 +- > > 3 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > index b5fadc2a48..65b8ed74ad 100644 > > --- a/doc/APIchanges > > +++ b/doc/APIchanges > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > > > API changes, most recent first: > > > > +2019-07-XX - XX - lavu 56.31.100 - mem.h > > + Add av_memcpy() > > + > > 2019-06-21 - XX - lavu 56.30.100 - frame.h > >Add FF_DECODE_ERROR_DECODE_SLICES > > > > diff --git a/libavutil/mem.h b/libavutil/mem.h > > index 5fb1a02dd9..a35230360d 100644 > > --- a/libavutil/mem.h > > +++ b/libavutil/mem.h > > @@ -506,6 +506,19 @@ void *av_memdup(const void *p, size_t size); > > */ > > void av_memcpy_backptr(uint8_t *dst, int back, int cnt); > > > > +/** > > + * memcpy() implementation without a NULL pointer special case > > + * > > + * @param dst Destination buffer > > + * @param src Source buffer > > + * @param cnt Number of bytes to copy; must be >= 0 > > + */ > > +static inline void av_memcpy(void *dst, const void *src, size_t cnt) > > How many cases are there in the codebase where cnt can be 0, and dst or > src NULL, without it having been checked before calling memcpy? And from > those, which would not be from situations where the code should have > instead aborted and returned ENOMEM, or EINVAL if either of them are > function arguments? There are around 2500 occurances of memcpy in the codebase To awnser your question it would be needed to review all of them and in many cases their surrounding code. So that is unlikely to be awnsered by anyone accuratly Also iam not sure i understand why you ask or why this would matter the suggested function allows to simplify cases where the NULL can occur, not where it cannot or should not. That is this is intended for the cases where we already have or are adding explicit checks to avoid the NULL case. i could rename this to av_memcpy_nullsafe which would make it clearer but also its more to write and read thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
Hello, thanks for review,
1) Can you give some reason about why we shouldn't use errno? I think it is
more clear to accept the full int32 range, even if min/max int32 values are
very unlikely to be used.
2) The RFC 4566 don't give any limit on fmtp parameters values. In addition
I can't find a spec that says fmtp integers parameters for AAC must be
positive. So I don't think we should limit values to positive integers here
as it would not be consistent to RFC.
On Sat, Jun 29, 2019 at 5:58 AM Reimar Döffinger
wrote:
> I don't think we should be using errno when avoidable, and it is avoidable
> here by disallowing min/max int32 values themselves. Or using strtoll.
> I'm also rather sceptical about allowing negative values here, does that
> make sense?
> Admittedly the type is set to just "int", but maybe it should be unsigned
> instead?
>
> On 28.06.2019, at 08:46, Olivier MAIGNIAL
> wrote:
>
> > Hello here!
> >
> > A simple ping about this patch
> > If you have any question, feel free to ask!
> >
> > Regards,
> > Olivier
> >
> > On Wed, Jun 19, 2019 at 3:38 PM Olivier Maignial <
> olivier.maign...@smile.fr>
> > wrote:
> >
> >> === PROBLEM ===
> >>
> >> I was trying to record h264 + aac streams from an RTSP server to mp4
> file.
> >> using this command line:
> >>ffmpeg -v verbose -y -i "rtsp:///my_resources" -codec copy -bsf:a
> >> aac_adtstoasc test.mp4
> >>
> >> FFmpeg then fail to record audio and output this logs:
> >>[rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40)
> >>[rtsp @ 0xcda1f0] Error parsing AU headers
> >>...
> >>[rtsp @ 0xcda1f0] Could not find codec parameters for stream 1
> (Audio:
> >> aac, 48000 Hz, 1 channels): unspecified sample format
> >>
> >> In SDP provided by my RTSP server I had this fmtp line:
> >>a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr;
> >> config=1188; sizeLength=13; indexLength=3; indexDeltaLength=3;
> >>
> >> In FFmpeg code, I found a check introduced by commit
> >> 24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater
> than
> >> 32 for fmtp line parameters.
> >> However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual
> Streams)
> >> give examples of "profile-level-id" values for AAC, up to 55.
> >> Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give
> any
> >> limit of size on interger parameters given in fmtp line.
> >>
> >> === FIX ===
> >>
> >> Instead of prohibit values over 32, I propose to check the possible
> >> integer overflow.
> >> The use of strtol allow to check the string validity and the possible
> >> overflow.
> >> Value is then checked against INT32_MIN and INT32_MAX. Using
> INT32_MIN/MAX
> >> ensure to have the same behavior on 32 or 64 bits platforms.
> >>
> >> This patch fix my problem and I now can record my RTSP AAC stream to
> mp4.
> >> It has passed the full fate tests suite sucessfully.
> >>
> >> Signed-off-by: Olivier Maignial
> >> ---
> >>
> >> Changes V5 -> V6:
> >>- Simplify code
> >>
> >> libavformat/rtpdec_mpeg4.c | 15 ++-
> >> 1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> >> index 4f70599..9c4f8a1 100644
> >> --- a/libavformat/rtpdec_mpeg4.c
> >> +++ b/libavformat/rtpdec_mpeg4.c
> >> @@ -289,15 +289,20 @@ static int parse_fmtp(AVFormatContext *s,
> >> for (i = 0; attr_names[i].str; ++i) {
> >> if (!av_strcasecmp(attr, attr_names[i].str)) {
> >> if (attr_names[i].type == ATTR_NAME_TYPE_INT) {
> >> -int val = atoi(value);
> >> -if (val > 32) {
> >> +char *end_ptr = NULL;
> >> +errno = 0;
> >> +long int val = strtol(value, _ptr, 10);
> >> +if (end_ptr == value || end_ptr[0] != '\0' ||
> >> +errno == ERANGE ||
> >> +val < INT32_MIN || val > INT32_MAX) {
> >> av_log(s, AV_LOG_ERROR,
> >> - "The %s field size is invalid (%d)\n",
> >> - attr, val);
> >> + "The %s field value is not a valid
> number,
> >> or overflows int32: %s\n",
> >> + attr, value);
> >> return AVERROR_INVALIDDATA;
> >> }
> >> +
> >> *(int *)((char *)data+
> >> -attr_names[i].offset) = val;
> >> +attr_names[i].offset) = (int) val;
> >> } else if (attr_names[i].type == ATTR_NAME_TYPE_STR) {
> >> char *val = av_strdup(value);
> >> if (!val)
> >> --
> >> 2.7.4
> >>
> >>
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above,
Re: [FFmpeg-devel] [PATCH v1] lavc/libdavs2.c: optimize frame copy
On Wed, Jul 3, 2019 at 11:24 AM hwrenx wrote:
>
> Signed-off-by: hwrenx
> ---
> libavcodec/libdavs2.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index 218f3ec..15ed3a1 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -62,7 +62,7 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> davs2_picture_t *pic, int *g
> davs2_seq_info_t *headerset, int ret_type,
> AVFrame *frame)
> {
> DAVS2Context *cad= avctx->priv_data;
> -int bytes_per_sample = pic->bytes_per_sample;
> +int bytes_per_sample = pic->bytes_per_sample == 8 ? 1 : 2;
Looks like davs2 use a wrong name for bits per sample, I think need to
fix in davs2 first, it's better than the workaround in the wrapper.
> int plane = 0;
> int line = 0;
>
> @@ -104,6 +104,7 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> davs2_picture_t *pic, int *g
>
> for (plane = 0; plane < 3; ++plane) {
> int size_line = pic->widths[plane] * bytes_per_sample;
> +void *dst, *src;
> frame->buf[plane] = av_buffer_alloc(size_line * pic->lines[plane]);
>
> if (!frame->buf[plane]){
> @@ -114,10 +115,14 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> davs2_picture_t *pic, int *g
> frame->data[plane] = frame->buf[plane]->data;
> frame->linesize[plane] = size_line;
>
> -for (line = 0; line < pic->lines[plane]; ++line)
> -memcpy(frame->data[plane] + line * size_line,
> - pic->planes[plane] + line * pic->strides[plane],
> - pic->widths[plane] * bytes_per_sample);
> +dst = frame->data[plane];
> +src = pic->planes[plane];
> +
> +for (line = 0; line < pic->lines[plane]; ++line) {
> +memcpy(dst, src, size_line);
> +dst += size_line;
> +src += pic->strides[plane];
> +}
> }
>
> frame->width = cad->headerset.width;
> --
> 2.7.4
>
___
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] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On 03.07.2019, at 08:29, Michael Niedermayer wrote: > On Tue, Jul 02, 2019 at 08:42:43PM -0300, James Almer wrote: >> On 7/2/2019 5:56 PM, Michael Niedermayer wrote: >>> Signed-off-by: Michael Niedermayer >>> --- >>> doc/APIchanges | 3 +++ >>> libavutil/mem.h | 13 + >>> libavutil/version.h | 2 +- >>> 3 files changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/doc/APIchanges b/doc/APIchanges >>> index b5fadc2a48..65b8ed74ad 100644 >>> --- a/doc/APIchanges >>> +++ b/doc/APIchanges >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 >>> >>> API changes, most recent first: >>> >>> +2019-07-XX - XX - lavu 56.31.100 - mem.h >>> + Add av_memcpy() >>> + >>> 2019-06-21 - XX - lavu 56.30.100 - frame.h >>> Add FF_DECODE_ERROR_DECODE_SLICES >>> >>> diff --git a/libavutil/mem.h b/libavutil/mem.h >>> index 5fb1a02dd9..a35230360d 100644 >>> --- a/libavutil/mem.h >>> +++ b/libavutil/mem.h >>> @@ -506,6 +506,19 @@ void *av_memdup(const void *p, size_t size); >>> */ >>> void av_memcpy_backptr(uint8_t *dst, int back, int cnt); >>> >>> +/** >>> + * memcpy() implementation without a NULL pointer special case >>> + * >>> + * @param dst Destination buffer >>> + * @param src Source buffer >>> + * @param cnt Number of bytes to copy; must be >= 0 >>> + */ >>> +static inline void av_memcpy(void *dst, const void *src, size_t cnt) >> >> How many cases are there in the codebase where cnt can be 0, and dst or >> src NULL, without it having been checked before calling memcpy? And from >> those, which would not be from situations where the code should have >> instead aborted and returned ENOMEM, or EINVAL if either of them are >> function arguments? > > There are around 2500 occurances of memcpy in the codebase > To awnser your question it would be needed to review all of them and in many > cases their surrounding code. > So that is unlikely to be awnsered by anyone accuratly > > Also iam not sure i understand why you ask or why this would matter > the suggested function allows to simplify cases where the NULL can > occur, not where it cannot or should not. That is this is intended for > the cases where we already have or are adding explicit checks to > avoid the NULL case. > > i could rename this to av_memcpy_nullsafe which would make it clearer but > also its more to write and read I admit I thought that a worthwhile idea originally, but I have to think back to a long time ago that every function added to our "API" has a cost of people having to know about it and how to use it. And if it's currently only 2 places that would benefit I think James is right to ask if it makes sense. Of course another question might be if it might make sense to replace all memcpy uses with it. I mean, isn't it naturally expected behaviour that the pointers would be ignored if the copy amount is 0? There might be a lot of code assuming that we do not know about... ___ 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] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On Wed, Jul 03, 2019 at 09:41:41AM +0200, Reimar Döffinger wrote: > > > On 03.07.2019, at 08:29, Michael Niedermayer wrote: > > > On Tue, Jul 02, 2019 at 08:42:43PM -0300, James Almer wrote: > >> On 7/2/2019 5:56 PM, Michael Niedermayer wrote: > >>> Signed-off-by: Michael Niedermayer > >>> --- > >>> doc/APIchanges | 3 +++ > >>> libavutil/mem.h | 13 + > >>> libavutil/version.h | 2 +- > >>> 3 files changed, 17 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/doc/APIchanges b/doc/APIchanges > >>> index b5fadc2a48..65b8ed74ad 100644 > >>> --- a/doc/APIchanges > >>> +++ b/doc/APIchanges > >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > >>> > >>> API changes, most recent first: > >>> > >>> +2019-07-XX - XX - lavu 56.31.100 - mem.h > >>> + Add av_memcpy() > >>> + > >>> 2019-06-21 - XX - lavu 56.30.100 - frame.h > >>> Add FF_DECODE_ERROR_DECODE_SLICES > >>> > >>> diff --git a/libavutil/mem.h b/libavutil/mem.h > >>> index 5fb1a02dd9..a35230360d 100644 > >>> --- a/libavutil/mem.h > >>> +++ b/libavutil/mem.h > >>> @@ -506,6 +506,19 @@ void *av_memdup(const void *p, size_t size); > >>> */ > >>> void av_memcpy_backptr(uint8_t *dst, int back, int cnt); > >>> > >>> +/** > >>> + * memcpy() implementation without a NULL pointer special case > >>> + * > >>> + * @param dst Destination buffer > >>> + * @param src Source buffer > >>> + * @param cnt Number of bytes to copy; must be >= 0 > >>> + */ > >>> +static inline void av_memcpy(void *dst, const void *src, size_t cnt) > >> > >> How many cases are there in the codebase where cnt can be 0, and dst or > >> src NULL, without it having been checked before calling memcpy? And from > >> those, which would not be from situations where the code should have > >> instead aborted and returned ENOMEM, or EINVAL if either of them are > >> function arguments? > > > > There are around 2500 occurances of memcpy in the codebase > > To awnser your question it would be needed to review all of them and in many > > cases their surrounding code. > > So that is unlikely to be awnsered by anyone accuratly > > > > Also iam not sure i understand why you ask or why this would matter > > the suggested function allows to simplify cases where the NULL can > > occur, not where it cannot or should not. That is this is intended for > > the cases where we already have or are adding explicit checks to > > avoid the NULL case. > > > > i could rename this to av_memcpy_nullsafe which would make it clearer but > > also its more to write and read > > I admit I thought that a worthwhile idea originally, > but I have to think back to a long time ago that every function added to our > "API" has a cost of people having to know about it and how to use it. > And if it's currently only 2 places that would benefit I think James is right > to ask if it makes sense. > Of course another question might be if it might make sense to replace all > memcpy uses with it. > I mean, isn't it naturally expected behaviour that the pointers would be > ignored if the copy amount is 0? There might be a lot of code assuming that > we do not know about... in addition to the 2 there are these related commits found by very dumb git log greps In further addition there would be cases that deal with src == dst, something we could add a check for in av_memcpy() too commit c6aaf0840cf9b2b8cb139ed7110d3d47c2bf3d12 Author: Carl Eugen Hoyos Date: Tue Apr 18 10:56:31 2017 +0200 lavf/mov: Only copy extradata if it exists. Avoids undefined call of memcpy(ptr, NULL, 0); commit fde9013ab42411ee2015811c28e8921828a81702 Author: Derek Buitenhuis Date: Thu Jul 6 13:23:06 2017 -0400 mpegtsenc: Don't pass NULL to memcpy Signed-off-by: Derek Buitenhuis commit 7bab631f7df55b361368296f125b95a1814bc18c Author: Michael Niedermayer Date: Wed Mar 6 01:37:49 2013 +0100 mss2dsp/upsample_plane: fix 0x0 handling Fixes invalid memcpy and out of array accesses Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind Signed-off-by: Michael Niedermayer commit c54eef46f990722ed65fd1ad1da3d0fc50806eb5 Author: Carl Eugen Hoyos Date: Thu Sep 22 01:03:55 2016 +0200 lavc/avpacket: Fix undefined behaviour, do not pass a null pointer to memcpy(). Fixes ticket #5857. commit f077ad69c682c13ab75a72aec11a61cac53f0c91 Author: Carl Eugen Hoyos Date: Sun Sep 4 21:11:02 2016 +0200 lavc/avpacket: Fix undefined behaviour, do not pass a null pointer to memcpy(). Fixes ticket #5128. -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above,
[FFmpeg-devel] movenc track_duration
Hi, here is my attempt to fix this bug: https://trac.ffmpeg.org/ticket/7966 I am wondering if track_duration should include the last duration or not. I tried to include duration in the calculation of track_duration here: diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 46d314ff17..6cd209266d 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -5612,7 +5612,7 @@ static int mov_write_single_packet(AVFormatContext *s, AVPacket *pkt) // sample in this track. This avoids relying on AVPacket // duration, but only helps for this particular track, not // for the other ones that are flushed at the same time. -trk->track_duration = pkt->dts - trk->start_dts; +trk->track_duration = pkt->dts - trk->start_dts + pkt->duration; if (pkt->pts != AV_NOPTS_VALUE) trk->end_pts = pkt->pts; else ... but this adds a warning in the testprogram: (same warning as the ticket) ./libavformat/tests/movenc [mp4 @ 0x7f8ae0801200] track 1: codec frame size is not set write_data len 36, time nopts, type header atom ftyp write_data len 2397, time nopts, type header atom - [mp4 @ 0x7f8ae0801200] Application provided duration: -512 / timestamp: 15360 is out of range for mov/mp4 format [mp4 @ 0x7f8ae0801200] pts has no value write_data len 908, time 103, type sync atom moof write_data len 110, time nopts, type trailer atom - 45218facb761a83f686e2363d4f971f4 3451 non-empty-moov [mp4 @ 0x7f8ae0801200] track 1: codec frame size is not set write_data len 36, time nopts, type header atom ftyp write_data len 2729, time nopts, type header atom - [mp4 @ 0x7f8ae0801200] Application provided duration: -512 / timestamp: 14848 is out of range for mov/mp4 format [mp4 @ 0x7f8ae0801200] pts has no value write_data len 1028, time 100, type sync atom moof write_data len 110, time nopts, type trailer atom - cf7b45d08c99438d0d55dd2ec28f0fe5 3903 non-empty-moov-elst [mp4 @ 0x7f8ae0801200] track 1: codec frame size is not set write_data len 36, time nopts, type header atom ftyp write_data len 2637, time nopts, type header atom - [mp4 @ 0x7f8ae0801200] Application provided duration: -512 / timestamp: 15360 is out of range for mov/mp4 format [mp4 @ 0x7f8ae0801200] pts has no value write_data len 1028, time 103, type sync atom moof write_data len 110, time nopts, type trailer atom - fea7eec4e4d6f0f72a38fe6ea49fac9a 3811 non-empty-moov-no-elst so the bug can be easily provoked with this change. in my humble opinion the check is too strict. /alfred ___ 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] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On Wed, Jul 3, 2019 at 10:46 AM Michael Niedermayer wrote: > > On Wed, Jul 03, 2019 at 09:41:41AM +0200, Reimar Döffinger wrote: > > > > > > On 03.07.2019, at 08:29, Michael Niedermayer wrote: > > > > > On Tue, Jul 02, 2019 at 08:42:43PM -0300, James Almer wrote: > > >> On 7/2/2019 5:56 PM, Michael Niedermayer wrote: > > >>> Signed-off-by: Michael Niedermayer > > >>> --- > > >>> doc/APIchanges | 3 +++ > > >>> libavutil/mem.h | 13 + > > >>> libavutil/version.h | 2 +- > > >>> 3 files changed, 17 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/doc/APIchanges b/doc/APIchanges > > >>> index b5fadc2a48..65b8ed74ad 100644 > > >>> --- a/doc/APIchanges > > >>> +++ b/doc/APIchanges > > >>> @@ -15,6 +15,9 @@ libavutil: 2017-10-21 > > >>> > > >>> API changes, most recent first: > > >>> > > >>> +2019-07-XX - XX - lavu 56.31.100 - mem.h > > >>> + Add av_memcpy() > > >>> + > > >>> 2019-06-21 - XX - lavu 56.30.100 - frame.h > > >>> Add FF_DECODE_ERROR_DECODE_SLICES > > >>> > > >>> diff --git a/libavutil/mem.h b/libavutil/mem.h > > >>> index 5fb1a02dd9..a35230360d 100644 > > >>> --- a/libavutil/mem.h > > >>> +++ b/libavutil/mem.h > > >>> @@ -506,6 +506,19 @@ void *av_memdup(const void *p, size_t size); > > >>> */ > > >>> void av_memcpy_backptr(uint8_t *dst, int back, int cnt); > > >>> > > >>> +/** > > >>> + * memcpy() implementation without a NULL pointer special case > > >>> + * > > >>> + * @param dst Destination buffer > > >>> + * @param src Source buffer > > >>> + * @param cnt Number of bytes to copy; must be >= 0 > > >>> + */ > > >>> +static inline void av_memcpy(void *dst, const void *src, size_t cnt) > > >> > > >> How many cases are there in the codebase where cnt can be 0, and dst or > > >> src NULL, without it having been checked before calling memcpy? And from > > >> those, which would not be from situations where the code should have > > >> instead aborted and returned ENOMEM, or EINVAL if either of them are > > >> function arguments? > > > > > > There are around 2500 occurances of memcpy in the codebase > > > To awnser your question it would be needed to review all of them and in > > > many > > > cases their surrounding code. > > > So that is unlikely to be awnsered by anyone accuratly > > > > > > Also iam not sure i understand why you ask or why this would matter > > > the suggested function allows to simplify cases where the NULL can > > > occur, not where it cannot or should not. That is this is intended for > > > the cases where we already have or are adding explicit checks to > > > avoid the NULL case. > > > > > > i could rename this to av_memcpy_nullsafe which would make it clearer but > > > also its more to write and read > > > > I admit I thought that a worthwhile idea originally, > > but I have to think back to a long time ago that every function added to > > our "API" has a cost of people having to know about it and how to use it. > > And if it's currently only 2 places that would benefit I think James is > > right to ask if it makes sense. > > Of course another question might be if it might make sense to replace all > > memcpy uses with it. > > I mean, isn't it naturally expected behaviour that the pointers would be > > ignored if the copy amount is 0? There might be a lot of code assuming that > > we do not know about... > > in addition to the 2 there are these related commits found by very dumb git > log greps > In further addition there would be cases that deal with src == dst, something > we > could add a check for in av_memcpy() too > Personally I really don't like hiding too much magic in a function like this. It can easily lead to brittle code, someone may think the pointer is always valid since its memcpy'ed to, and uses it afterwards, and there you have a disaster. Either the function name should make perfectly clear what it does, or preferably it should just not exist and code should just validate its stuff. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] Add 2 timestamp print formats
On Tue, Jul 02, 2019 at 12:28:53AM +0200, Ulf Zibis wrote:
> Thanks Michael for your review, answers inline ...
>
> Am 01.07.19 um 17:16 schrieb Michael Niedermayer:
> >> timestamp.h | 78
> >>
> >> 1 file changed, 73 insertions(+), 5 deletions(-)
> >> 3d1275421b1b8d4952815151158c7af954d38ca6 timestamp_2.patch
> >> From 709cb4662132deff2d17a8700f58fa91b118c56d Mon Sep 17 00:00:00 2001
> >> From: Ulf Zibis
> >> Date: 29.06.2019, 17:52:06
> >>
> >> avutil/timestamp: added 2 new print formats
> >>
> >> diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h
> >> index e082f01..b94e5ce 100644
> >> --- a/libavutil/timestamp.h
> >> +++ b/libavutil/timestamp.h
> >> @@ -48,14 +48,14 @@
> >> }
> >>
> >> /**
> >> - * Convenience macro, the return value should be used only directly in
> >> + * Convenience macro. The return value should be used only directly in
> >> * function arguments but never stand-alone.
> >> */
> >> -#define av_ts2str(ts) av_ts_make_string((char[AV_TS_MAX_STRING_SIZE]){0},
> >> ts)
> >> +#define av_ts2str(ts)
> >> av_ts_make_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts)
> >>
> >> /**
> >> * Fill the provided buffer with a string containing a timestamp time
> >> - * representation.
> >> + * representation in seconds.
> >> *
> >> * @param buf a buffer with size in bytes of at least
> >> AV_TS_MAX_STRING_SIZE
> >> * @param ts the timestamp to represent
> >> @@ -70,9 +70,77 @@
> >> }
> >>
> >> /**
> >> - * Convenience macro, the return value should be used only directly in
> >> + * Convenience macro. The return value should be used only directly in
> >> * function arguments but never stand-alone.
> >> */
> > Unrelated change, belongs in a seperate patch
> The following change I think should be part of the patch, as it helps to
> distinguish between the existing timestamp functions and my new ones:
> - * representation.
> + * representation in seconds.
> The other above changes are cosmetic and can go in a separate patch. But
> would you endorse them?
Iam not a english composer ...
>
> >> -#define av_ts2timestr(ts, tb)
> >> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb)
> >> +#define av_ts2timestr(ts, tb)
> >> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
> >> +
> >> +/**
> >> + * Fill the provided buffer with a string containing a timestamp time
> >> + * representation in minutes and seconds.
> >> + *
> >> + * @param buf a buffer with size in bytes of at least
> >> AV_TS_MAX_STRING_SIZE
> >> + * @param ts the timestamp to represent
> >> + * @param tb the timebase of the timestamp
> >> + * @return the buffer in input
> >> + */
> >> +static inline char *av_ts_make_minute_string(char *buf, int64_t ts,
> >> AVRational *tb)
> >> +{
> >> +if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE,
> >> "NOPTS");
> >> +else {
> >> +double time = av_q2d(*tb) * ts;
> > If this could be done without float/double that would be preferred as it
> > avoids inaccuracies / slight differences between platforms
>
> I too thought on that, but the existing functions too rely on
> float/double. Should I anyway provide a solution with integer arithmetic?
its indepedant of your patch but i think all these should use integers
unless its too messy
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Observe your enemies, for they first find out your faults. -- Antisthenes
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avutil: add av_memcpy() to avoid undefined behavior with NULL, NULL, 0
On 03/07/2019 08:41, Reimar Döffinger wrote: > Of course another question might be if it might make sense to replace all > memcpy uses with it. I would expect this may break some compiler optimizations around memcpy (like __builtin_memcpy, or direct inlining), no? - Derek ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v4] avfilter/avf_aphasemeter: Add out-of-phase and mono detection
I've added documentation for the extension of aphasemeter filter.
Also, I'm not sure that "phasing" is the right word to describe the
detection.
From 1e356929e878a2081add102b77a9560647232ef8 Mon Sep 17 00:00:00 2001
From: Romane Lafon
Date: Wed, 3 Jul 2019 15:15:16 +0200
Subject: [PATCH] avfilter/avf_aphasemeter: Add out of phase and mono detection
Signed-off-by: Romane Lafon
---
doc/filters.texi | 32 +++
libavfilter/avf_aphasemeter.c | 127 --
2 files changed, 155 insertions(+), 4 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 700a76f239..ec8c73d558 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -20656,6 +20656,38 @@ Set color which will be used for drawing median phase. If color is
Enable video output. Default is enabled.
@end table
+@subsection phasing detection
+
+The filter also detects out of phase and mono sequences in stereo streams.
+It logs the sequence start, end and duration when it lasts longer or as long as the minimum set.
+
+The filter accepts the following options for this detection:
+
+@table @option
+@item phasing
+Enable mono and out of phase detection. Default is disabled.
+
+@item tolerance
+Set phase tolerance for mono detection, in amplitude ratio. Default is @code{0}.
+Allowed range is @code{[0, 1]}.
+
+@item angle
+Set angle threshold for out of phase detection, in degree. Default is @code{170}.
+Allowed range is @code{[0, 180]}.
+
+@item duration
+Set mono or out of phase duration until notification, expressed in seconds. Default is @code{2}.
+
+@subsection Examples
+
+@itemize
+@item
+Complete example with @command{ffmpeg} to detect 1 second of mono with 0.001 phase tolerance:
+@example
+ffmpeg -i stereo.wav -af aphasemeter=video=0:phasing=1:duration=1:tolerance=0.001 -f null -
+@end example
+@end itemize
+
@section avectorscope
Convert input audio to a video output, representing the audio vector
diff --git a/libavfilter/avf_aphasemeter.c b/libavfilter/avf_aphasemeter.c
index f497bc9969..77701e5cde 100644
--- a/libavfilter/avf_aphasemeter.c
+++ b/libavfilter/avf_aphasemeter.c
@@ -28,26 +28,41 @@
#include "libavutil/intreadwrite.h"
#include "libavutil/opt.h"
#include "libavutil/parseutils.h"
+#include "libavutil/timestamp.h"
#include "avfilter.h"
#include "formats.h"
#include "audio.h"
#include "video.h"
#include "internal.h"
+#include "stdbool.h"
+#include "float.h"
typedef struct AudioPhaseMeterContext {
const AVClass *class;
AVFrame *out;
int do_video;
+int do_phasing_detection;
int w, h;
AVRational frame_rate;
int contrast[4];
uint8_t *mpc_str;
uint8_t mpc[4];
int draw_median_phase;
+int is_mono;
+int is_out_phase;
+int start_mono_presence;
+int start_out_phase_presence;
+float tolerance;
+float angle;
+float phase;
+float mono_idx[2];
+float out_phase_idx[2];
+double duration;
} AudioPhaseMeterContext;
#define OFFSET(x) offsetof(AudioPhaseMeterContext, x)
#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
+#define get_duration(index) (index[1] - index[0])
static const AVOption aphasemeter_options[] = {
{ "rate", "set video rate", OFFSET(frame_rate), AV_OPT_TYPE_VIDEO_RATE, {.str="25"}, 0, INT_MAX, FLAGS },
@@ -59,6 +74,10 @@ static const AVOption aphasemeter_options[] = {
{ "bc", "set blue contrast", OFFSET(contrast[2]), AV_OPT_TYPE_INT, {.i64=1}, 0, 255, FLAGS },
{ "mpc", "set median phase color", OFFSET(mpc_str), AV_OPT_TYPE_STRING, {.str = "none"}, 0, 0, FLAGS },
{ "video", "set video output", OFFSET(do_video), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, FLAGS },
+{ "phasing", "set mono and out-of-phase detection output", OFFSET(do_phasing_detection), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS },
+{ "tolerance", "set phase tolerance for mono detection", OFFSET(tolerance), AV_OPT_TYPE_FLOAT, {.dbl = 0.}, 0, 1, FLAGS },
+{ "angle", "set angle threshold for out-of-phase detection", OFFSET(angle), AV_OPT_TYPE_FLOAT, {.dbl = 170.}, 90, 180, FLAGS },
+{ "duration", "set minimum mono or out-of-phase duration in seconds", OFFSET(duration), AV_OPT_TYPE_DOUBLE, {.dbl=2.}, 0, 24*60*60, FLAGS },
{ NULL }
};
@@ -140,6 +159,22 @@ static inline int get_x(float phase, int w)
return (phase + 1.) / 2. * (w - 1);
}
+static inline float get_index(AVFilterLink *inlink, AVFrame *in)
+{
+char *index_str = av_ts2timestr(in->pts, >time_base);
+return atof(index_str);
+}
+
+static inline void add_metadata(AVFrame *insamples, const char *key, float value)
+{
+char buf[128];
+char str[128];
+
+snprintf(str, sizeof(str), "%f", value);
+snprintf(buf, sizeof(buf), "lavfi.aphasemeter.%s", key);
+av_dict_set(>metadata, buf, str, 0);
+}
+
static int filter_frame(AVFilterLink *inlink, AVFrame *in)
{
AVFilterContext *ctx = inlink->dst;
@@ -154,6 +189,10 @@ static int filter_frame(AVFilterLink *inlink,
Re: [FFmpeg-devel] [PATCH v4] Add 2 timestamp print formats
Am 03.07.19 um 10:52 schrieb Michael Niedermayer:
>
-#define av_ts2timestr(ts, tb)
av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb)
+#define av_ts2timestr(ts, tb)
av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
+
+/**
+ * Fill the provided buffer with a string containing a timestamp time
+ * representation in minutes and seconds.
+ *
+ * @param buf a buffer with size in bytes of at least
AV_TS_MAX_STRING_SIZE
+ * @param ts the timestamp to represent
+ * @param tb the timebase of the timestamp
+ * @return the buffer in input
+ */
+static inline char *av_ts_make_minute_string(char *buf, int64_t ts,
AVRational *tb)
+{
+if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE,
"NOPTS");
+else {
+double time = av_q2d(*tb) * ts;
>>> If this could be done without float/double that would be preferred as it
>>> avoids inaccuracies / slight differences between platforms
>> I too thought on that, but the existing functions too rely on
>> float/double. Should I anyway provide a solution with integer arithmetic?
> its indepedant of your patch but i think all these should use integers
> unless its too messy
Thanks for you opinion.
Here comes a new patch.
-Ulf
From 5d62406366560cfab5711120c514a77867bd8c2e Mon Sep 17 00:00:00 2001
From: Ulf Zibis
Date: 29.06.2019, 17:52:06
avutil/timestamp: added av_ts2us() and 2 new print formats
diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h
index e082f01..8cb6e8f 100644
--- a/libavutil/timestamp.h
+++ b/libavutil/timestamp.h
@@ -33,6 +33,17 @@
#define AV_TS_MAX_STRING_SIZE 32
/**
+ * Convert a time base scaled timestamp to micro seconds.
+ *
+ * @param ts the timestamp to convert, must be less than 2**63/1,000,000/tb->num
+ * @param tb the timebase of the timestamp
+ * @return the timestamp in micro seconds
+ */
+static inline int64_t av_ts2us(int64_t ts, AVRational *tb) {
+return ts * 100 * tb->num / tb->den;
+}
+
+/**
* Fill the provided buffer with a string containing a timestamp
* representation.
*
@@ -55,7 +66,7 @@
/**
* Fill the provided buffer with a string containing a timestamp time
- * representation.
+ * representation in seconds.
*
* @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
* @param ts the timestamp to represent
@@ -75,4 +86,60 @@
*/
#define av_ts2timestr(ts, tb) av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb)
+/**
+ * Fill the provided buffer with a string containing a timestamp time
+ * representation in minutes and seconds.
+ *
+ * @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
+ * @param ts the timestamp to represent
+ * @param tb the timebase of the timestamp
+ * @return the buffer in input
+ */
+static char *av_ts_make_minute_string(char *buf, int64_t ts, AVRational *tb)
+{
+if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
+else {
+int64_t us = av_ts2us(ts, tb);
+int len = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%3ld:%02d.%06d",
+us / 6000, (int)(us / 100 % 60), (int)(us % 100));
+while (buf[--len] == '0'); // search trailing zeros or ...
+buf[len + (buf[len] != '.')] = '\0'; // dot and strip them
+}
+return buf;
+}
+
+/**
+ * Convenience macro. The return value should be used only directly in
+ * function arguments but never stand-alone.
+ */
+#define av_ts2minutestr(ts, tb) av_ts_make_minute_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
+
+/**
+ * Fill the provided buffer with a string containing a timestamp time
+ * representation in hours, minutes and seconds.
+ *
+ * @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
+ * @param ts the timestamp to represent
+ * @param tb the timebase of the timestamp
+ * @return the buffer in input
+ */
+static char *av_ts_make_hour_string(char *buf, int64_t ts, AVRational *tb)
+{
+if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
+else {
+int64_t us = av_ts2us(ts, tb);
+int len = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%ld:%02d:%02d.%06d",
+us / 36, (int)(us / 6000 % 60), (int)(us / 100 % 60), (int)(us % 100));
+while (buf[--len] == '0'); // search trailing zeros or ...
+buf[len + (buf[len] != '.')] = '\0'; // dot and strip them
+}
+return buf;
+}
+
+/**
+ * Convenience macro. The return value should be used only directly in
+ * function arguments but never stand-alone.
+ */
+#define av_ts2hourstr(ts, tb) av_ts_make_hour_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
+
#endif /* AVUTIL_TIMESTAMP_H */
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
[FFmpeg-devel] [PATCH] Ensure scaled video is divisible by n
This patch adds a new option to the scale filter which ensures that the
output resolution is divisible by the given integer similar to using -n
in the `w` and `h` options. But this works even if the
`force_original_aspect_ratio` is used.
The use case for this is to set a fixed target resolution using `w` and
`h`, to use the `force_original_aspect_ratio` option to make sure that
the video always fits in the defined bounding box regardless of aspect
ratio, but to also make sure that the calculated output resolution is
divisible by n so in can be encoded with certain encoders/options if
that is required.
Signed-off-by: Lars Kiesow
---
doc/filters.texi | 5 +
libavfilter/vf_scale.c | 9 ++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 700a76f239..1694fdda28 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -15215,6 +15215,11 @@ Please note that this is a different thing than
specifying -1 for @option{w}
or @option{h}, you still need to specify the output resolution for this option
to work.
+@item force_divisible_by
+Ensures that the output resolution is divisible by the given integer similar
+to using -n in the @option{w} and @option{h} options. But this works even if
+the @option{force_original_aspect_ratio} is used.
+
@end table
The values of the @option{w} and @option{h} options are expressions
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f741419e7e..1cc28f6a56 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -86,6 +86,7 @@ typedef struct ScaleContext {
int in_v_chr_pos;
int force_original_aspect_ratio;
+int force_divisible_by;
int nb_slices;
@@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
goto fail;
/* Note that force_original_aspect_ratio may overwrite the previous set
- * dimensions so that it is not divisible by the set factors anymore. */
+ * dimensions so that it is not divisible by the set factors anymore
+ * unless force_divisible_by is defined as well */
if (scale->force_original_aspect_ratio) {
-int tmp_w = av_rescale(h, inlink->w, inlink->h);
-int tmp_h = av_rescale(w, inlink->h, inlink->w);
+int tmp_w = av_rescale(h, inlink->w, inlink->h) /
scale->force_divisible_by * scale->force_divisible_by;
+int tmp_h = av_rescale(w, inlink->h, inlink->w) /
scale->force_divisible_by * scale->force_divisible_by;
if (scale->force_original_aspect_ratio == 1) {
w = FFMIN(tmp_w, w);
@@ -589,6 +591,7 @@ static const AVOption scale_options[] = {
{ "out_v_chr_pos", "output vertical chroma position in luma grid/256" ,
OFFSET(out_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
{ "out_h_chr_pos", "output horizontal chroma position in luma grid/256",
OFFSET(out_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
{ "force_original_aspect_ratio", "decrease or increase w/h if necessary to
keep the original AR", OFFSET(force_original_aspect_ratio), AV_OPT_TYPE_INT, {
.i64 = 0}, 0, 2, FLAGS, "force_oar" },
+{ "force_divisible_by", "endorce that the output resolution is devisible
by a defined integer", OFFSET(force_divisible_by), AV_OPT_TYPE_INT, { .i64 =
1}, 0, 256, FLAGS, "force_oar" },
{ "disable", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0 }, 0, 0, FLAGS,
"force_oar" },
{ "decrease", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1 }, 0, 0, FLAGS,
"force_oar" },
{ "increase", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 2 }, 0, 0, FLAGS,
"force_oar" },
--
2.21.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v6] Fix integer parameters size check in SDP fmtp line
On 03.07.2019, at 09:28, Olivier MAIGNIAL wrote: > Hello, thanks for review, > > 1) Can you give some reason about why we shouldn't use errno? I think it is > more clear to accept the full int32 range, even if min/max int32 values are > very unlikely to be used. It is a global variable, with all the issues that has, also for code clarity (to review code you need to know which functions modify it). On some systems it is not even thread-local, so using it becomes completely unsafe once threads are involved, but even when it is, signal handlers can by accident change it. I see the argument that these are rare, special and broken cases, but it's not like using errno gains all that much (esp. when using stroll is also an alternative that avoids the need). The shorter version of that argument is that errno is a major misdesign beyond even strcpy and strncpy levels, which I personally find good reason to avoid it where reasonable. Note that there is also the issue that "long" is 32 bit on 32 bit systems, I would expect the if condition to trigger compiler warnings there, which strtoll should avoid. > 2) The RFC 4566 don't give any limit on fmtp parameters values. In addition > I can't find a spec that says fmtp integers parameters for AAC must be > positive. So I don't think we should limit values to positive integers here > as it would not be consistent to RFC. I admit I have not found any requirement on it being integer at all, so not sure if that assumption is not wrong to start with and should at least not trigger an error. And nothing that would make > 32 bit wrong. ___ 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] Ensure scaled video is divisible by n
On 7/3/19, Lars Kiesow wrote:
> This patch adds a new option to the scale filter which ensures that the
> output resolution is divisible by the given integer similar to using -n
> in the `w` and `h` options. But this works even if the
> `force_original_aspect_ratio` is used.
>
> The use case for this is to set a fixed target resolution using `w` and
> `h`, to use the `force_original_aspect_ratio` option to make sure that
> the video always fits in the defined bounding box regardless of aspect
> ratio, but to also make sure that the calculated output resolution is
> divisible by n so in can be encoded with certain encoders/options if
> that is required.
>
> Signed-off-by: Lars Kiesow
> ---
> doc/filters.texi | 5 +
> libavfilter/vf_scale.c | 9 ++---
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 700a76f239..1694fdda28 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -15215,6 +15215,11 @@ Please note that this is a different thing than
> specifying -1 for @option{w}
> or @option{h}, you still need to specify the output resolution for this
> option
> to work.
>
> +@item force_divisible_by
> +Ensures that the output resolution is divisible by the given integer
> similar
> +to using -n in the @option{w} and @option{h} options. But this works even
> if
> +the @option{force_original_aspect_ratio} is used.
> +
> @end table
>
> The values of the @option{w} and @option{h} options are expressions
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index f741419e7e..1cc28f6a56 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -86,6 +86,7 @@ typedef struct ScaleContext {
> int in_v_chr_pos;
>
> int force_original_aspect_ratio;
> +int force_divisible_by;
>
> int nb_slices;
>
> @@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
> goto fail;
>
> /* Note that force_original_aspect_ratio may overwrite the previous set
> - * dimensions so that it is not divisible by the set factors anymore.
> */
> + * dimensions so that it is not divisible by the set factors anymore
> + * unless force_divisible_by is defined as well */
> if (scale->force_original_aspect_ratio) {
> -int tmp_w = av_rescale(h, inlink->w, inlink->h);
> -int tmp_h = av_rescale(w, inlink->h, inlink->w);
> +int tmp_w = av_rescale(h, inlink->w, inlink->h) /
> scale->force_divisible_by * scale->force_divisible_by;
> +int tmp_h = av_rescale(w, inlink->h, inlink->w) /
> scale->force_divisible_by * scale->force_divisible_by;
>
> if (scale->force_original_aspect_ratio == 1) {
> w = FFMIN(tmp_w, w);
> @@ -589,6 +591,7 @@ static const AVOption scale_options[] = {
> { "out_v_chr_pos", "output vertical chroma position in luma grid/256"
> , OFFSET(out_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS
> },
> { "out_h_chr_pos", "output horizontal chroma position in luma
> grid/256", OFFSET(out_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513,
> 512, FLAGS },
> { "force_original_aspect_ratio", "decrease or increase w/h if necessary
> to keep the original AR", OFFSET(force_original_aspect_ratio),
> AV_OPT_TYPE_INT, { .i64 = 0}, 0, 2, FLAGS, "force_oar" },
> +{ "force_divisible_by", "endorce that the output resolution is
> devisible by a defined integer", OFFSET(force_divisible_by),
endorce and devisible are typos.
> AV_OPT_TYPE_INT, { .i64 = 1}, 0, 256, FLAGS, "force_oar" },
> { "disable", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0 }, 0, 0, FLAGS,
> "force_oar" },
> { "decrease", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1 }, 0, 0, FLAGS,
> "force_oar" },
> { "increase", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 2 }, 0, 0, FLAGS,
> "force_oar" },
> --
> 2.21.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Ensure scaled video is divisible by n
This patch adds a new option to the scale filter which ensures that the
output resolution is divisible by the given integer similar to using -n
in the `w` and `h` options. But this works even if the
`force_original_aspect_ratio` is used.
The use case for this is to set a fixed target resolution using `w` and
`h`, to use the `force_original_aspect_ratio` option to make sure that
the video always fits in the defined bounding box regardless of aspect
ratio, but to also make sure that the calculated output resolution is
divisible by n so in can be encoded with certain encoders/options if
that is required.
Signed-off-by: Lars Kiesow
---
doc/filters.texi | 5 +
libavfilter/vf_scale.c | 9 ++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 700a76f239..1694fdda28 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -15215,6 +15215,11 @@ Please note that this is a different thing than
specifying -1 for @option{w}
or @option{h}, you still need to specify the output resolution for this option
to work.
+@item force_divisible_by
+Ensures that the output resolution is divisible by the given integer similar
+to using -n in the @option{w} and @option{h} options. But this works even if
+the @option{force_original_aspect_ratio} is used.
+
@end table
The values of the @option{w} and @option{h} options are expressions
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f741419e7e..c02b82dd07 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -86,6 +86,7 @@ typedef struct ScaleContext {
int in_v_chr_pos;
int force_original_aspect_ratio;
+int force_divisible_by;
int nb_slices;
@@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
goto fail;
/* Note that force_original_aspect_ratio may overwrite the previous set
- * dimensions so that it is not divisible by the set factors anymore. */
+ * dimensions so that it is not divisible by the set factors anymore
+ * unless force_divisible_by is defined as well */
if (scale->force_original_aspect_ratio) {
-int tmp_w = av_rescale(h, inlink->w, inlink->h);
-int tmp_h = av_rescale(w, inlink->h, inlink->w);
+int tmp_w = av_rescale(h, inlink->w, inlink->h) /
scale->force_divisible_by * scale->force_divisible_by;
+int tmp_h = av_rescale(w, inlink->h, inlink->w) /
scale->force_divisible_by * scale->force_divisible_by;
if (scale->force_original_aspect_ratio == 1) {
w = FFMIN(tmp_w, w);
@@ -589,6 +591,7 @@ static const AVOption scale_options[] = {
{ "out_v_chr_pos", "output vertical chroma position in luma grid/256" ,
OFFSET(out_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
{ "out_h_chr_pos", "output horizontal chroma position in luma grid/256",
OFFSET(out_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
{ "force_original_aspect_ratio", "decrease or increase w/h if necessary to
keep the original AR", OFFSET(force_original_aspect_ratio), AV_OPT_TYPE_INT, {
.i64 = 0}, 0, 2, FLAGS, "force_oar" },
+{ "force_divisible_by", "enforce that the output resolution is divisible
by a defined integer", OFFSET(force_divisible_by), AV_OPT_TYPE_INT, { .i64 =
1}, 0, 256, FLAGS, "force_oar" },
{ "disable", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0 }, 0, 0, FLAGS,
"force_oar" },
{ "decrease", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1 }, 0, 0, FLAGS,
"force_oar" },
{ "increase", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 2 }, 0, 0, FLAGS,
"force_oar" },
--
2.21.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] Ensure scaled video is divisible by n
On 7/3/19, Lars Kiesow wrote:
> This patch adds a new option to the scale filter which ensures that the
> output resolution is divisible by the given integer similar to using -n
> in the `w` and `h` options. But this works even if the
> `force_original_aspect_ratio` is used.
>
> The use case for this is to set a fixed target resolution using `w` and
> `h`, to use the `force_original_aspect_ratio` option to make sure that
> the video always fits in the defined bounding box regardless of aspect
> ratio, but to also make sure that the calculated output resolution is
> divisible by n so in can be encoded with certain encoders/options if
> that is required.
>
> Signed-off-by: Lars Kiesow
> ---
> doc/filters.texi | 5 +
> libavfilter/vf_scale.c | 9 ++---
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 700a76f239..1694fdda28 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -15215,6 +15215,11 @@ Please note that this is a different thing than
> specifying -1 for @option{w}
> or @option{h}, you still need to specify the output resolution for this
> option
> to work.
>
> +@item force_divisible_by
> +Ensures that the output resolution is divisible by the given integer
> similar
> +to using -n in the @option{w} and @option{h} options. But this works even
> if
> +the @option{force_original_aspect_ratio} is used.
> +
> @end table
>
> The values of the @option{w} and @option{h} options are expressions
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index f741419e7e..c02b82dd07 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -86,6 +86,7 @@ typedef struct ScaleContext {
> int in_v_chr_pos;
>
> int force_original_aspect_ratio;
> +int force_divisible_by;
>
> int nb_slices;
>
> @@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
> goto fail;
>
> /* Note that force_original_aspect_ratio may overwrite the previous set
> - * dimensions so that it is not divisible by the set factors anymore.
> */
> + * dimensions so that it is not divisible by the set factors anymore
> + * unless force_divisible_by is defined as well */
> if (scale->force_original_aspect_ratio) {
> -int tmp_w = av_rescale(h, inlink->w, inlink->h);
> -int tmp_h = av_rescale(w, inlink->h, inlink->w);
> +int tmp_w = av_rescale(h, inlink->w, inlink->h) /
> scale->force_divisible_by * scale->force_divisible_by;
> +int tmp_h = av_rescale(w, inlink->h, inlink->w) /
> scale->force_divisible_by * scale->force_divisible_by;
>
> if (scale->force_original_aspect_ratio == 1) {
> w = FFMIN(tmp_w, w);
> @@ -589,6 +591,7 @@ static const AVOption scale_options[] = {
> { "out_v_chr_pos", "output vertical chroma position in luma grid/256"
> , OFFSET(out_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS
> },
> { "out_h_chr_pos", "output horizontal chroma position in luma
> grid/256", OFFSET(out_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513,
> 512, FLAGS },
> { "force_original_aspect_ratio", "decrease or increase w/h if necessary
> to keep the original AR", OFFSET(force_original_aspect_ratio),
> AV_OPT_TYPE_INT, { .i64 = 0}, 0, 2, FLAGS, "force_oar" },
> +{ "force_divisible_by", "enforce that the output resolution is
> divisible by a defined integer", OFFSET(force_divisible_by),
> AV_OPT_TYPE_INT, { .i64 = 1}, 0, 256, FLAGS, "force_oar" },
Are you sure you need "force_oar" ?
If not, move it bellow 3 lines.
> { "disable", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0 }, 0, 0, FLAGS,
> "force_oar" },
> { "decrease", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1 }, 0, 0, FLAGS,
> "force_oar" },
> { "increase", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 2 }, 0, 0, FLAGS,
> "force_oar" },
> --
> 2.21.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel 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] Ensure scaled video is divisible by n
Hi Paul,
> > { "force_original_aspect_ratio", "decrease or increase w/h if
> > necessary to keep the original AR",
> > OFFSET(force_original_aspect_ratio), AV_OPT_TYPE_INT, { .i64 = 0},
> > 0, 2, FLAGS, "force_oar" },
> > +{ "force_divisible_by", "enforce that the output resolution is
> > divisible by a defined integer", OFFSET(force_divisible_by),
> > AV_OPT_TYPE_INT, { .i64 = 1}, 0, 256, FLAGS, "force_oar" },
>
> Are you sure you need "force_oar" ?
> If not, move it bellow 3 lines.
I'm honestly not sure since I don't know what it's used for ;-)
From the AVOption Struct docs, I assumed that this should have the same
value as in force_original_aspect_ratio since it's used in the same
context. But it does work without and I can remove it if you want me to.
Should I?
Best regards,
Lars
___
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] Ensure scaled video is divisible by n
On 7/3/19, Lars Kiesow wrote:
> Hi Paul,
>
>> > { "force_original_aspect_ratio", "decrease or increase w/h if
>> > necessary to keep the original AR",
>> > OFFSET(force_original_aspect_ratio), AV_OPT_TYPE_INT, { .i64 = 0},
>> > 0, 2, FLAGS, "force_oar" },
>> > +{ "force_divisible_by", "enforce that the output resolution is
>> > divisible by a defined integer", OFFSET(force_divisible_by),
>> > AV_OPT_TYPE_INT, { .i64 = 1}, 0, 256, FLAGS, "force_oar" },
>>
>> Are you sure you need "force_oar" ?
>> If not, move it bellow 3 lines.
>
> I'm honestly not sure since I don't know what it's used for ;-)
>
> From the AVOption Struct docs, I assumed that this should have the same
> value as in force_original_aspect_ratio since it's used in the same
> context. But it does work without and I can remove it if you want me to.
> Should I?
Yes, Look at belllow 3 lines, they have "force_oar" at end, because
that gives special meaning to values.
Your option should not have these.
To confirm these, run "ffmpeg -h filter=scale" before and after you removed it.
>
> Best regards,
> Lars
>
___
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] Ensure scaled video is divisible by n
On Wed, 3 Jul 2019 18:04:26 +0200
Paul B Mahol wrote:
> On 7/3/19, Lars Kiesow wrote:
> > Hi Paul,
> >
> >> > { "force_original_aspect_ratio", "decrease or increase w/h
> >> > if necessary to keep the original AR",
> >> > OFFSET(force_original_aspect_ratio), AV_OPT_TYPE_INT, { .i64 =
> >> > 0}, 0, 2, FLAGS, "force_oar" },
> >> > +{ "force_divisible_by", "enforce that the output resolution
> >> > is divisible by a defined integer", OFFSET(force_divisible_by),
> >> > AV_OPT_TYPE_INT, { .i64 = 1}, 0, 256, FLAGS, "force_oar" },
> >>
> >> Are you sure you need "force_oar" ?
> >> If not, move it bellow 3 lines.
> >
> > I'm honestly not sure since I don't know what it's used for ;-)
> >
> > From the AVOption Struct docs, I assumed that this should have the
> > same value as in force_original_aspect_ratio since it's used in the
> > same context. But it does work without and I can remove it if you
> > want me to. Should I?
>
> Yes, Look at belllow 3 lines, they have "force_oar" at end, because
> that gives special meaning to values.
>
> Your option should not have these.
>
> To confirm these, run "ffmpeg -h filter=scale" before and after you
> removed it.
Oh, got it makes sense. Thanks.
Will update the patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] Ensure scaled video is divisible by n
This patch adds a new option to the scale filter which ensures that the
output resolution is divisible by the given integer similar to using -n
in the `w` and `h` options. But this works even if the
`force_original_aspect_ratio` is used.
The use case for this is to set a fixed target resolution using `w` and
`h`, to use the `force_original_aspect_ratio` option to make sure that
the video always fits in the defined bounding box regardless of aspect
ratio, but to also make sure that the calculated output resolution is
divisible by n so in can be encoded with certain encoders/options if
that is required.
Signed-off-by: Lars Kiesow
---
doc/filters.texi | 5 +
libavfilter/vf_scale.c | 9 ++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 700a76f239..1694fdda28 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -15215,6 +15215,11 @@ Please note that this is a different thing than
specifying -1 for @option{w}
or @option{h}, you still need to specify the output resolution for this option
to work.
+@item force_divisible_by
+Ensures that the output resolution is divisible by the given integer similar
+to using -n in the @option{w} and @option{h} options. But this works even if
+the @option{force_original_aspect_ratio} is used.
+
@end table
The values of the @option{w} and @option{h} options are expressions
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f741419e7e..d1b486f3d6 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -86,6 +86,7 @@ typedef struct ScaleContext {
int in_v_chr_pos;
int force_original_aspect_ratio;
+int force_divisible_by;
int nb_slices;
@@ -237,10 +238,11 @@ static int config_props(AVFilterLink *outlink)
goto fail;
/* Note that force_original_aspect_ratio may overwrite the previous set
- * dimensions so that it is not divisible by the set factors anymore. */
+ * dimensions so that it is not divisible by the set factors anymore
+ * unless force_divisible_by is defined as well */
if (scale->force_original_aspect_ratio) {
-int tmp_w = av_rescale(h, inlink->w, inlink->h);
-int tmp_h = av_rescale(w, inlink->h, inlink->w);
+int tmp_w = av_rescale(h, inlink->w, inlink->h) /
scale->force_divisible_by * scale->force_divisible_by;
+int tmp_h = av_rescale(w, inlink->h, inlink->w) /
scale->force_divisible_by * scale->force_divisible_by;
if (scale->force_original_aspect_ratio == 1) {
w = FFMIN(tmp_w, w);
@@ -592,6 +594,7 @@ static const AVOption scale_options[] = {
{ "disable", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 0 }, 0, 0, FLAGS,
"force_oar" },
{ "decrease", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 1 }, 0, 0, FLAGS,
"force_oar" },
{ "increase", NULL, 0, AV_OPT_TYPE_CONST, {.i64 = 2 }, 0, 0, FLAGS,
"force_oar" },
+{ "force_divisible_by", "enforce that the output resolution is divisible
by a defined integer", OFFSET(force_divisible_by), AV_OPT_TYPE_INT, { .i64 =
1}, 1, 256, FLAGS },
{ "param0", "Scaler param 0", OFFSET(param[0]),
AV_OPT_TYPE_DOUBLE, { .dbl = SWS_PARAM_DEFAULT }, INT_MIN, INT_MAX, FLAGS },
{ "param1", "Scaler param 1", OFFSET(param[1]),
AV_OPT_TYPE_DOUBLE, { .dbl = SWS_PARAM_DEFAULT }, INT_MIN, INT_MAX, FLAGS },
{ "nb_slices", "set the number of slices (debug purpose only)",
OFFSET(nb_slices), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, FLAGS },
--
2.21.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] avformat/rpl: Replace strcpy with av_strlcpy
The patch should be applied on top of the Replay ADPCM patches. Regards Cameron On Mon, 1 Jul 2019 at 15:07, Michael Niedermayer wrote: > On Sun, Jun 30, 2019 at 12:00:43AM +0100, Cameron Cawley wrote: > > --- > > libavformat/rpl.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > this doesnt apply, against which branch is that patch ? > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > It is what and why we do it that matters, not just one of them. > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4] Add 2 timestamp print formats
Ulf Zibis (12019-07-03): > From 5d62406366560cfab5711120c514a77867bd8c2e Mon Sep 17 00:00:00 2001 > From: Ulf Zibis > Date: 29.06.2019, 17:52:06 > > avutil/timestamp: added av_ts2us() and 2 new print formats The features you add here seem useful, but I wonder: have you considered the option of making them a single function with a parameter to select the format? That may make it easier to add new formats later, and have them supported by any component where this is relevant. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v4] Add 2 timestamp print formats
Am 03.07.19 um 16:49 schrieb Ulf Zibis:
> Am 03.07.19 um 10:52 schrieb Michael Niedermayer:
> -#define av_ts2timestr(ts, tb)
> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb)
> +#define av_ts2timestr(ts, tb)
> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
> +
> +/**
> + * Fill the provided buffer with a string containing a timestamp time
> + * representation in minutes and seconds.
> + *
> + * @param buf a buffer with size in bytes of at least
> AV_TS_MAX_STRING_SIZE
> + * @param ts the timestamp to represent
> + * @param tb the timebase of the timestamp
> + * @return the buffer in input
> + */
> +static inline char *av_ts_make_minute_string(char *buf, int64_t ts,
> AVRational *tb)
> +{
> +if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE,
> "NOPTS");
> +else {
> +double time = av_q2d(*tb) * ts;
If this could be done without float/double that would be preferred as it
avoids inaccuracies / slight differences between platforms
>>> I too thought on that, but the existing functions too rely on
>>> float/double. Should I anyway provide a solution with integer arithmetic?
>> its indepedant of your patch but i think all these should use integers
>> unless its too messy
> Thanks for you opinion.
>
> Here comes a new patch.
I missed to mark the big integer literal with 'L' and add "must be less
than 2**63/1,000,000/tb->num" to all 3 functions. So I attach a correction.
-Ulf
>From e8c42ed84492d495e506da30dc7163edafccc9e4 Mon Sep 17 00:00:00 2001
From: Ulf Zibis
Date: 29.06.2019, 17:52:06
avutil/timestamp: 2 new print formats with av_ts2us()
diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h
index e082f01..f442fc0 100644
--- a/libavutil/timestamp.h
+++ b/libavutil/timestamp.h
@@ -33,6 +33,17 @@
#define AV_TS_MAX_STRING_SIZE 32
/**
+ * Convert a time base scaled timestamp to micro seconds.
+ *
+ * @param ts the timestamp to convert, must be less than 2**63/1,000,000/tb->num
+ * @param tb the timebase of the timestamp
+ * @return the timestamp in micro seconds
+ */
+static inline int64_t av_ts2us(int64_t ts, AVRational *tb) {
+return ts * 100 * tb->num / tb->den;
+}
+
+/**
* Fill the provided buffer with a string containing a timestamp
* representation.
*
@@ -55,7 +66,7 @@
/**
* Fill the provided buffer with a string containing a timestamp time
- * representation.
+ * representation in seconds.
*
* @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
* @param ts the timestamp to represent
@@ -75,4 +86,60 @@
*/
#define av_ts2timestr(ts, tb) av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb)
+/**
+ * Fill the provided buffer with a string containing a timestamp time
+ * representation in minutes and seconds.
+ *
+ * @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
+ * @param ts the timestamp to represent, must be less than 2**63/1,000,000/tb->num
+ * @param tb the timebase of the timestamp
+ * @return the buffer in input
+ */
+static char *av_ts_make_minute_string(char *buf, int64_t ts, AVRational *tb)
+{
+if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
+else {
+int64_t us = av_ts2us(ts, tb);
+int len = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%3ld:%02d.%06d",
+us / 6000, (int)(us / 100 % 60), (int)(us % 100));
+while (buf[--len] == '0'); // search trailing zeros or ...
+buf[len + (buf[len] != '.')] = '\0'; // dot and strip them
+}
+return buf;
+}
+
+/**
+ * Convenience macro. The return value should be used only directly in
+ * function arguments but never stand-alone.
+ */
+#define av_ts2minutestr(ts, tb) av_ts_make_minute_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb)
+
+/**
+ * Fill the provided buffer with a string containing a timestamp time
+ * representation in hours, minutes and seconds.
+ *
+ * @param buf a buffer with size in bytes of at least AV_TS_MAX_STRING_SIZE
+ * @param ts the timestamp to represent, must be less than 2**63/1,000,000/tb->num
+ * @param tb the timebase of the timestamp
+ * @return the buffer in input
+ */
+static char *av_ts_make_hour_string(char *buf, int64_t ts, AVRational *tb)
+{
+if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, "NOPTS");
+else {
+int64_t us = av_ts2us(ts, tb);
+int len = snprintf(buf, AV_TS_MAX_STRING_SIZE, "%ld:%02d:%02d.%06d",
+us / 36L, (int)(us / 6000 % 60), (int)(us / 100 % 60), (int)(us % 100));
+while (buf[--len] == '0'); // search trailing zeros or ...
+buf[len + (buf[len] != '.')] = '\0'; // dot and strip them
+}
+return buf;
+}
+
+/**
+ * Convenience macro. The return value should be used only directly in
+ * function arguments but never stand-alone.
+ */
+#define av_ts2hourstr(ts, tb)
Re: [FFmpeg-devel] [PATCH v4] Add 2 timestamp print formats
Am 03.07.19 um 16:52 schrieb Nicolas George: > The features you add here seem useful, but I wonder: have you considered > the option of making them a single function with a parameter to select > the format? That may make it easier to add new formats later, and have > them supported by any component where this is relevant. I didn't have considerd that. If one of the project managers will agree to, please make a suggestion for the parameters you want, and I can do an additional patch. -Ulf ___ 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] libavfilter/vf_colorspace.c: fix demarcation point of gamma linearize function
On Wed, Jul 3, 2019 at 12:51 AM vincenluo(罗永林) wrote: > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Great find, your patch is absolutely correct. Pushing soon, thank you. -- Vittorio ___ 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] movenc: fix warning if CONFIG_AC3_PARSER not defined
On Tue, Jul 02, 2019 at 02:19:37PM +0200, Alfred E. Heggestad wrote: > this patch fixes a compiler warning if CONFIG_AC3_PARSER is > not defined. The label 'end' is removed and all the code > use the label 'err' instead. What compiler warning (this should be in the commit message) "goto err" in the case where its not an error would require a comment Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH]lavc/vc2enc_dwt: Avoid left-shifting a negative value
On Tue, Jul 02, 2019 at 11:40:52AM +0200, Carl Eugen Hoyos wrote: > Hi! > > Attached patch intends to fix ticket #7985. > > Please comment, Carl Eugen > vc2enc_dwt.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > dad3681a3b8ddce4b2a1aa81795d042b99069117 > 0001-lavc-vc2enc_dwt-Avoid-left-shifting-a-negative-value.patch > From 740f1a95336b70ff3f8e6d46f0c61115890ee3e7 Mon Sep 17 00:00:00 2001 > From: Carl Eugen Hoyos > Date: Tue, 2 Jul 2019 11:38:14 +0200 > Subject: [PATCH] lavc/vc2enc_dwt: Avoid left-shifting a negative value. > > Fixes ticket #7985. > --- > libavcodec/vc2enc_dwt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) LGTM but iam not the maintainer of this thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] MAINTAINERS: add myself to the AMF section
NAK for reasons said on IRC ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/2] avcodec/huffyuvdec: Check vertical subsampling in hymt
Fixes: out of array access
Fixes:
15484/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HYMT_fuzzer-5765377054736384
Fixes:
15559/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HYMT_fuzzer-5710295743332352
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/huffyuvdec.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 771481fd03..46dcfa8235 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -1252,6 +1252,7 @@ static int decode_frame(AVCodecContext *avctx, void
*data, int *got_frame,
slice_height = AV_RL32(avpkt->data + buf_size - 8);
nb_slices = AV_RL32(avpkt->data + buf_size - 12);
if (nb_slices * 8LL + slices_info_offset > buf_size - 16 ||
+s->chroma_v_shift ||
slice_height <= 0 || nb_slices * (uint64_t)slice_height > height)
return AVERROR_INVALIDDATA;
} else {
--
2.22.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/2] avcodec/huffyuv: remove gray8a (the format is listed but not supported by the implementation)
Fixes: null pointer dereference
Fixes:
15464/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HYMT_fuzzer-5681391150301184
Found-by: continuous fuzzing process
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer
---
libavcodec/huffyuvdec.c | 3 ---
libavcodec/huffyuvenc.c | 2 --
2 files changed, 5 deletions(-)
diff --git a/libavcodec/huffyuvdec.c b/libavcodec/huffyuvdec.c
index 27f650d7bf..771481fd03 100644
--- a/libavcodec/huffyuvdec.c
+++ b/libavcodec/huffyuvdec.c
@@ -418,9 +418,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
case 0x0F0:
avctx->pix_fmt = AV_PIX_FMT_GRAY16;
break;
-case 0x170:
-avctx->pix_fmt = AV_PIX_FMT_GRAY8A;
-break;
case 0x470:
avctx->pix_fmt = AV_PIX_FMT_GBRP;
break;
diff --git a/libavcodec/huffyuvenc.c b/libavcodec/huffyuvenc.c
index 3662c173ec..a6f0d06445 100644
--- a/libavcodec/huffyuvenc.c
+++ b/libavcodec/huffyuvenc.c
@@ -268,7 +268,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
case AV_PIX_FMT_YUVA420P:
case AV_PIX_FMT_YUVA422P:
case AV_PIX_FMT_GBRAP:
-case AV_PIX_FMT_GRAY8A:
case AV_PIX_FMT_YUV420P9:
case AV_PIX_FMT_YUV420P10:
case AV_PIX_FMT_YUV420P12:
@@ -1122,7 +1121,6 @@ AVCodec ff_ffvhuff_encoder = {
AV_PIX_FMT_GRAY8, AV_PIX_FMT_GRAY16,
AV_PIX_FMT_YUVA420P, AV_PIX_FMT_YUVA422P, AV_PIX_FMT_YUVA444P,
AV_PIX_FMT_GBRAP,
-AV_PIX_FMT_GRAY8A,
AV_PIX_FMT_YUV420P9, AV_PIX_FMT_YUV420P10, AV_PIX_FMT_YUV420P12,
AV_PIX_FMT_YUV420P14, AV_PIX_FMT_YUV420P16,
AV_PIX_FMT_YUV422P9, AV_PIX_FMT_YUV422P10, AV_PIX_FMT_YUV422P12,
AV_PIX_FMT_YUV422P14, AV_PIX_FMT_YUV422P16,
AV_PIX_FMT_YUV444P9, AV_PIX_FMT_YUV444P10, AV_PIX_FMT_YUV444P12,
AV_PIX_FMT_YUV444P14, AV_PIX_FMT_YUV444P16,
--
2.22.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] MAINTAINERS: add myself to the AMF section
--- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 88b0109f22..a66e4fc338 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -138,6 +138,7 @@ Codecs: aacenc*, aaccoder.c Rostislav Pehlivanov alacenc.c Jaikrishnan Menon alsdec.c Thilo Borgmann, Umair Khan + amf* Alexander Kravchenko aptx.cAurelien Jacobs ass* Aurelien Jacobs asv* Michael Niedermayer -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: Integrate two checks
On Mon, Jul 01, 2019 at 11:04:43PM +0200, Andreas Rheinhardt wrote:
> For audio packets with dts != AV_NOPTS_VALUE the dts the dts was
> converted twice to the muxer's timebase during streamcopy, once as a
> normal packet and once specifically as an audio packet. This has been
> changed.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> You are right. Seems that this is not covered by fate.
>
> fftools/ffmpeg.c | 19 +--
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 01f04103cf..f81b1b3983 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2044,20 +2044,19 @@ static void do_streamcopy(InputStream *ist,
> OutputStream *ost, const AVPacket *p
> else
> opkt.pts = AV_NOPTS_VALUE;
>
> -if (pkt->dts == AV_NOPTS_VALUE)
> +if (pkt->dts == AV_NOPTS_VALUE) {
> opkt.dts = av_rescale_q(ist->dts, AV_TIME_BASE_Q, ost->mux_timebase);
> -else
> -opkt.dts = av_rescale_q(pkt->dts, ist->st->time_base,
> ost->mux_timebase);
> -opkt.dts -= ost_tb_start_time;
> -
> -if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && pkt->dts !=
> AV_NOPTS_VALUE) {
> +} else if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> int duration = av_get_audio_frame_duration(ist->dec_ctx, pkt->size);
> if(!duration)
> duration = ist->dec_ctx->frame_size;
> -opkt.dts = opkt.pts = av_rescale_delta(ist->st->time_base, pkt->dts,
> - (AVRational){1,
> ist->dec_ctx->sample_rate}, duration, >filter_in_rescale_delta_last,
> - ost->mux_timebase) -
> ost_tb_start_time;
> -}
> +opkt.dts = av_rescale_delta(ist->st->time_base, pkt->dts,
> +(AVRational){1,
> ist->dec_ctx->sample_rate}, duration,
> +>filter_in_rescale_delta_last,
> ost->mux_timebase);
> +opkt.pts = opkt.dts - ost_tb_start_time;
this is ugly and confusing. As this introduces a difference betweem pts and dts
just because the adjustment for dts is factored out into a few lines later
> +} else
> +opkt.dts = av_rescale_q(pkt->dts, ist->st->time_base,
> ost->mux_timebase);
> +opkt.dts -= ost_tb_start_time;
>
> opkt.duration = av_rescale_q(pkt->duration, ist->st->time_base,
> ost->mux_timebase);
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH]lavc/vc2enc_dwt: Avoid left-shifting a negative value
Am Mi., 3. Juli 2019 um 21:24 Uhr schrieb Michael Niedermayer : > > On Tue, Jul 02, 2019 at 11:40:52AM +0200, Carl Eugen Hoyos wrote: > > Hi! > > > > Attached patch intends to fix ticket #7985. > > > > Please comment, Carl Eugen > > > vc2enc_dwt.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > dad3681a3b8ddce4b2a1aa81795d042b99069117 > > 0001-lavc-vc2enc_dwt-Avoid-left-shifting-a-negative-value.patch > > From 740f1a95336b70ff3f8e6d46f0c61115890ee3e7 Mon Sep 17 00:00:00 2001 > > From: Carl Eugen Hoyos > > Date: Tue, 2 Jul 2019 11:38:14 +0200 > > Subject: [PATCH] lavc/vc2enc_dwt: Avoid left-shifting a negative value. > > > > Fixes ticket #7985. > > --- > > libavcodec/vc2enc_dwt.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > LGTM but iam not the maintainer of this I'll wait for the final colour of the memcpy hut anyway;-) Carl Eugen ___ 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] MAINTAINERS: add myself to the AMF section
On Tue, 02 Jul 2019 17:19:02 +0300 Alexander Kravchenko wrote: > LGTM, but please try and get git send-email working. Your client is still doing binary attachments. --phil ___ 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] MAINTAINERS: add myself to the AMF section
On Thu, Jul 4, 2019 at 12:42 AM Lynne wrote: > > NAK for reasons said on IRC For everyones benefit, why don't you actually formulate your reasons here instead of asking people to piece them together from some chat history, that way people can actually understand or respond to them. I, for one, could not find an actual reason you listed that actually applies. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] MAINTAINERS: add myself to the AMF section
I am OK with Alex Kravchenko taking the official ownership of AMF integration. I am still involved in AMD AMF implementation and monitor FFmpeg changes. Mikhail Mironov From: ffmpeg-devel on behalf of Alexander Kravchenko Sent: Wednesday, July 3, 2019 6:53 PM To: ffmpeg-devel@ffmpeg.org Cc: Alexander Kravchenko Subject: [FFmpeg-devel] [PATCH] MAINTAINERS: add myself to the AMF section ___ 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".
