Re: [FFmpeg-devel] 4.4 Release Name
Stop raising tensions here. On Sat, Apr 3, 2021 at 10:07 PM Kieran Kunhya wrote: > Hey code of conduct committee, can we ban this person? > > Kieran > > Sent from my mobile device > > On Sat, 3 Apr 2021, 22:06 RADSL, wrote: > > > > > On 4/3/2021 12:47 PM, Kieran Kunhya wrote: > > > Who are you and what on earth are you talking about? > > > > > > Why on earth is it appropriate to name an ffmpeg release after a > disease > > > that has killed millions? > > > > > > Kieran > > > > > > Sent from my mobile device > > > > > Is it appropriate to make sport shoes with human blood? > > ___ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [RFC] CFHD
On Sat, Apr 3, 2021 at 1:01 PM Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-04-02 20:27:24) > > Hi all > > > > CFHD currently has even with all fixes (ignoring ones with objections) > applied a null pointer > > read and out of array write issue remaining. > > > > My patch which makes the header parsing more restrictive has an objection > > against it. and the only other developer who recently worked on it > > stated that he has no "time or motivation to deal with this and similar > issues" > > IMO any objection to a patch that does not include a clearly spelled out > reason for the objection and/or an alternative solution should be > ignored or regarded as spamming the ML. Especially when the patch > addresses crashes or other security issues. > Nice, You also prefer non-really security fixes. Good job team. I'm not going to repeat what I said about original patch(es). > -- > Anton Khirnov > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/cfhd: Check transform_type consistently
As always, make sure that you do not break or add new regressions for decoding existing valid files. On Sat, Apr 3, 2021 at 4:54 PM Michael Niedermayer wrote: > On Sat, Apr 03, 2021 at 04:39:06PM +0200, Michael Niedermayer wrote: > > Fixes: out of array accesses > > Fixes: > 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560 > > Fixes: > 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512 > > Fixes: > 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592 > > > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/cfhd.c | 11 +-- > > libavcodec/cfhd.h | 1 + > > 2 files changed, 10 insertions(+), 2 deletions(-) > > I intend to apply this patchset soon. Also this patchset almost certainly > does not fix every issue in CFHD, so if someone is searching for code to > do a security review on. CFHD is likely an interresting candidate > > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > it is not once nor twice but times without number that the same ideas make > their appearance in the world. -- Aristotle > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avfilter/vf_find_rect: Use correct format specifier
Fixes the following GCC warning:
warning: format ‘%lld’ expects argument of type ‘long long int’,
but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
Signed-off-by: Andreas Rheinhardt
---
Will apply soon.
libavfilter/vf_find_rect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavfilter/vf_find_rect.c b/libavfilter/vf_find_rect.c
index b6f5a1be29..5f3abf66c6 100644
--- a/libavfilter/vf_find_rect.c
+++ b/libavfilter/vf_find_rect.c
@@ -217,7 +217,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
}
}
-av_log(ctx, AV_LOG_INFO, "Found at n=%lld pts_time=%f x=%d y=%d with
score=%f\n",
+av_log(ctx, AV_LOG_INFO, "Found at n=%"PRId64" pts_time=%f x=%d y=%d with
score=%f\n",
inlink->frame_count_out, TS2D(in->pts) * av_q2d(inlink->time_base),
best_x, best_y, best_score);
foc->last_x = best_x;
--
2.27.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] avfilter/vf_find_rect: Use correct format specifier
On 2021-04-04 15:01, Andreas Rheinhardt wrote:
Fixes the following GCC warning:
warning: format ‘%lld’ expects argument of type ‘long long int’,
but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
Weird. I switched to lld because gcc (10.2) recommended it.
In fact, I just looked through the build log for my Windows build
compiled minutes ago, and there's no warning.
Regards,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_find_rect: Use correct format specifier
Gyan Doshi (12021-04-04):
> > Fixes the following GCC warning:
> > warning: format ‘%lld’ expects argument of type ‘long long int’,
> > but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
>
> Weird. I switched to lld because gcc (10.2) recommended it.
>
> In fact, I just looked through the build log for my Windows build compiled
> minutes ago, and there's no warning.
There is nothing weird about it: the int64_t type and its type
specifiers exist specifically because long and long long have different
meanings on different compilers.
If you are writing new C code with explicit references to long or short,
you are almost certainly doing it wrong.
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] avfilter/vf_find_rect: Use correct format specifier
Gyan Doshi:
>
>
> On 2021-04-04 15:01, Andreas Rheinhardt wrote:
>> Fixes the following GCC warning:
>> warning: format ‘%lld’ expects argument of type ‘long long int’,
>> but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
>
> Weird. I switched to lld because gcc (10.2) recommended it.
>
> In fact, I just looked through the build log for my Windows build
> compiled minutes ago, and there's no warning.
>
long int is 32bit on Windows, so int64_t can't be a typedef for long int
on said plattform; instead it is a typedef for long long int. That's why
one should use these PRIdx macros, because they will automatically be
converted to the correct system-dependent format specifier.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/vf_find_rect: Use correct format specifier
On 2021-04-04 15:29, Andreas Rheinhardt wrote:
Gyan Doshi:
On 2021-04-04 15:01, Andreas Rheinhardt wrote:
Fixes the following GCC warning:
warning: format ‘%lld’ expects argument of type ‘long long int’,
but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Wformat=]
Weird. I switched to lld because gcc (10.2) recommended it.
In fact, I just looked through the build log for my Windows build
compiled minutes ago, and there's no warning.
long int is 32bit on Windows, so int64_t can't be a typedef for long int
on said plattform; instead it is a typedef for long long int. That's why
one should use these PRIdx macros, because they will automatically be
converted to the correct system-dependent format specifier.
Ok. LGTM.
Regards,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Guo, Yejun (12021-04-01):
> > Is this allowed to be negative? Can/should this be size_t?
> There was a long discussion about size_t/int/uint32_t when I added
> struct AVRegionOfInterest in frame.h, and finally size_t is not chosen.
Then at least unsigned.
> yes, we can add a version number (enum AVBBoxHeaderVersion, see below) at the
> beginning of the struct, and the user needs to set it with
> AV_BBOX_HEADER_CURRENT.
> It is not elegant, is this what we really want?
A version number does not make the structure compatible. At best,
applications check the version and detect they do not support this one
and report an error. At worse, they do not bother to check and it causes
strange bugs. The version number does not help with compatibility, it
just makes it detectable.
To make the structure compatible, we have to ask ourselves: What happens
if we update it? What breaks compatibility? Once we have an answer, we
find a workaround.
In this case, what happens is that the variable array must be at the
end, and therefore its offset changes. And we cannot have a second
variable array (like the name! you had to set a constant size), and we
cannot update the type of the array elements.
And the solution becomes obvious: let us store the offsets in the
structure.
So, that would be:
typedef struct AVBoundingBoxHeader {
...
/**
* Offset of the array of bounding boxes.
*/
size_t bboxes_offset;
/**
* Offset increment in the array of bounding boxes.
*/
size_t bboxes_step;
};
Note that with that solution, we do not need the empty array, we can do
this:
AVBoundingBoxHeader *ff_bounding_box_alloc(size_t nb_bbox)
{
struct {
AVBoundingBoxHeader header;
AVBoundingBox boxes[nb_bbox];
} *ret;
ret = av_mallocz(sizeof(*ret));
/* add error checks */
ret->header->bboxes_offset = (char *)&ret->boxes - (char *)ret->header;
ret->header->bboxes_step = sizeof(*ret->boxes);
}
#define AV_BOUNDING_BOX_GET(header, idx) \
((AVBoundingBox *)((char *)(header) + (header)->bboxes_offset + (idx) *
(header)->bboxes_step))
You can do the same to lift the 128 limit on the name.
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] avformat/mpegtsenc: Preserve disposition in the absence of language
On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:
Implements ticket #9113.
Signed-off-by: Andreas Rheinhardt
---
libavformat/mpegtsenc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 35c835c484..dbd3bb148a 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
AVStream *st = s->streams[i];
MpegTSWriteStream *ts_st = st->priv_data;
AVDictionaryEntry *lang = av_dict_get(st->metadata, "language", NULL,
0);
+const char default_language[] = "und";
+const char *language = lang && strlen(lang->value) >= 3 ? lang->value
: default_language;
enum AVCodecID codec_id = st->codecpar->codec_id;
if (s->nb_programs) {
@@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
}
}
-if (lang) {
-char *p;
-char *next = lang->value;
+if (language != default_language ||
+st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS|
+ AV_DISPOSITION_HEARING_IMPAIRED |
+ AV_DISPOSITION_VISUAL_IMPAIRED)) {
+const char *p;
+const char *next = language;
uint8_t *len_ptr;
*q++ = ISO_639_LANGUAGE_DESCRIPTOR;
len_ptr = q++;
*len_ptr = 0;
-for (p = lang->value; next && *len_ptr < 255 / 4 * 4; p = next
+ 1) {
+for (p = next; next && *len_ptr < 255 / 4 * 4; p = next + 1) {
Maybe it would make the code more readable to do both initializations in
the for() statement, e.g.: for (p = next = language; ...)
LGTM otherwise.
Thanks,
Marton
if (q - data > SECTION_LENGTH - 4) {
err = 1;
break;
@@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
}
break;
case AVMEDIA_TYPE_SUBTITLE:
-{
- const char default_language[] = "und";
- const char *language = lang && strlen(lang->value) >= 3 ?
lang->value : default_language;
-
if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
uint8_t *len_ptr;
int extradata_copied = 0;
@@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
*len_ptr = q - len_ptr - 1;
}
-}
break;
case AVMEDIA_TYPE_VIDEO:
if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
--
2.27.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 V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
On 4/4/2021 7:01 AM, Nicolas George wrote:
Guo, Yejun (12021-04-01):
Is this allowed to be negative? Can/should this be size_t?
There was a long discussion about size_t/int/uint32_t when I added
struct AVRegionOfInterest in frame.h, and finally size_t is not chosen.
Then at least unsigned.
yes, we can add a version number (enum AVBBoxHeaderVersion, see below) at the
beginning of the struct, and the user needs to set it with
AV_BBOX_HEADER_CURRENT.
It is not elegant, is this what we really want?
A version number does not make the structure compatible. At best,
applications check the version and detect they do not support this one
and report an error. At worse, they do not bother to check and it causes
strange bugs. The version number does not help with compatibility, it
just makes it detectable.
To make the structure compatible, we have to ask ourselves: What happens
if we update it? What breaks compatibility? Once we have an answer, we
find a workaround.
In this case, what happens is that the variable array must be at the
end, and therefore its offset changes. And we cannot have a second
variable array (like the name! you had to set a constant size), and we
cannot update the type of the array elements.
And the solution becomes obvious: let us store the offsets in the
structure.
So, that would be:
typedef struct AVBoundingBoxHeader {
...
/**
* Offset of the array of bounding boxes.
*/
size_t bboxes_offset;
/**
* Offset increment in the array of bounding boxes.
*/
size_t bboxes_step;
};
Note that with that solution, we do not need the empty array, we can do
this:
AVBoundingBoxHeader *ff_bounding_box_alloc(size_t nb_bbox)
{
struct {
AVBoundingBoxHeader header;
AVBoundingBox boxes[nb_bbox];
} *ret;
ret = av_mallocz(sizeof(*ret));
/* add error checks */
ret->header->bboxes_offset = (char *)&ret->boxes - (char *)ret->header;
ret->header->bboxes_step = sizeof(*ret->boxes);
}
#define AV_BOUNDING_BOX_GET(header, idx) \
((AVBoundingBox *)((char *)(header) + (header)->bboxes_offset + (idx) *
(header)->bboxes_step))
This solution is what was used for video_enc_params.h, so i agree it
should be used here too. What's missing is a check for idx < nb_bbox
before accessing the offset in question, so an inline function instead
of a #define with a check for the above would be needed.
And since both are used as frame side data, it would be ideal that the
signature for the public helpers on both are the same (The standard
alloc, and the alloc + wrapping into frame side data ones).
You can do the same to lift the 128 limit on the name.
Regards,
___
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 V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Nicolas George:
> Guo, Yejun (12021-04-01):
>>> Is this allowed to be negative? Can/should this be size_t?
>> There was a long discussion about size_t/int/uint32_t when I added
>> struct AVRegionOfInterest in frame.h, and finally size_t is not chosen.
>
> Then at least unsigned.
>
>> yes, we can add a version number (enum AVBBoxHeaderVersion, see below) at the
>> beginning of the struct, and the user needs to set it with
>> AV_BBOX_HEADER_CURRENT.
>> It is not elegant, is this what we really want?
>
> A version number does not make the structure compatible. At best,
> applications check the version and detect they do not support this one
> and report an error. At worse, they do not bother to check and it causes
> strange bugs. The version number does not help with compatibility, it
> just makes it detectable.
>
> To make the structure compatible, we have to ask ourselves: What happens
> if we update it? What breaks compatibility? Once we have an answer, we
> find a workaround.
>
> In this case, what happens is that the variable array must be at the
> end, and therefore its offset changes. And we cannot have a second
> variable array (like the name! you had to set a constant size), and we
> cannot update the type of the array elements.
>
> And the solution becomes obvious: let us store the offsets in the
> structure.
>
> So, that would be:
>
> typedef struct AVBoundingBoxHeader {
> ...
> /**
>* Offset of the array of bounding boxes.
>*/
> size_t bboxes_offset;
>
> /**
>* Offset increment in the array of bounding boxes.
>*/
> size_t bboxes_step;
> };
>
> Note that with that solution, we do not need the empty array, we can do
> this:
>
> AVBoundingBoxHeader *ff_bounding_box_alloc(size_t nb_bbox)
> {
> struct {
> AVBoundingBoxHeader header;
> AVBoundingBox boxes[nb_bbox];
That's a variable-length array. That's a C99 feature we do not require.
> } *ret;
> ret = av_mallocz(sizeof(*ret));
> /* add error checks */
> ret->header->bboxes_offset = (char *)&ret->boxes - (char *)ret->header;
> ret->header->bboxes_step = sizeof(*ret->boxes);
> }
>
> #define AV_BOUNDING_BOX_GET(header, idx) \
> ((AVBoundingBox *)((char *)(header) + (header)->bboxes_offset + (idx) *
> (header)->bboxes_step))
>
> You can do the same to lift the 128 limit on the name.
>
> Regards,
>
>
> ___
> 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 V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
James Almer:
> On 4/4/2021 7:01 AM, Nicolas George wrote:
>> Guo, Yejun (12021-04-01):
Is this allowed to be negative? Can/should this be size_t?
>>> There was a long discussion about size_t/int/uint32_t when I added
>>> struct AVRegionOfInterest in frame.h, and finally size_t is not chosen.
>>
>> Then at least unsigned.
>>
>>> yes, we can add a version number (enum AVBBoxHeaderVersion, see
>>> below) at the
>>> beginning of the struct, and the user needs to set it with
>>> AV_BBOX_HEADER_CURRENT.
>>> It is not elegant, is this what we really want?
>>
>> A version number does not make the structure compatible. At best,
>> applications check the version and detect they do not support this one
>> and report an error. At worse, they do not bother to check and it causes
>> strange bugs. The version number does not help with compatibility, it
>> just makes it detectable.
>>
>> To make the structure compatible, we have to ask ourselves: What happens
>> if we update it? What breaks compatibility? Once we have an answer, we
>> find a workaround.
>>
>> In this case, what happens is that the variable array must be at the
>> end, and therefore its offset changes. And we cannot have a second
>> variable array (like the name! you had to set a constant size), and we
>> cannot update the type of the array elements.
>>
>> And the solution becomes obvious: let us store the offsets in the
>> structure.
>>
>> So, that would be:
>>
>> typedef struct AVBoundingBoxHeader {
>> ...
>> /**
>> * Offset of the array of bounding boxes.
>> */
>> size_t bboxes_offset;
>>
>> /**
>> * Offset increment in the array of bounding boxes.
>> */
>> size_t bboxes_step;
>> };
>>
>> Note that with that solution, we do not need the empty array, we can do
>> this:
>>
>> AVBoundingBoxHeader *ff_bounding_box_alloc(size_t nb_bbox)
>> {
>> struct {
>> AVBoundingBoxHeader header;
>> AVBoundingBox boxes[nb_bbox];
>> } *ret;
>> ret = av_mallocz(sizeof(*ret));
>> /* add error checks */
>> ret->header->bboxes_offset = (char *)&ret->boxes - (char
>> *)ret->header;
>> ret->header->bboxes_step = sizeof(*ret->boxes);
>> }
>>
>> #define AV_BOUNDING_BOX_GET(header, idx) \
>> ((AVBoundingBox *)((char *)(header) + (header)->bboxes_offset +
>> (idx) * (header)->bboxes_step))
>
> This solution is what was used for video_enc_params.h, so i agree it
> should be used here too. What's missing is a check for idx < nb_bbox
Actually nothing in video_enc_params.c guarantees proper alignment for
AVVideoBlockParams. If we added a type requiring 64bit alignment to
AVVideoBlockParams and are on a 32bit system, then the blocks will be
misaligned.
> before accessing the offset in question, so an inline function instead
> of a #define with a check for the above would be needed.
>
> And since both are used as frame side data, it would be ideal that the
> signature for the public helpers on both are the same (The standard
> alloc, and the alloc + wrapping into frame side data ones).
>
>>
>> You can do the same to lift the 128 limit on the name.
>>
>> Regards,
>>
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>>
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
James Almer (12021-04-04): > This solution is what was used for video_enc_params.h, so i agree it should > be used here too. What's missing is a check for idx < nb_bbox before > accessing the offset in question, so an inline function instead of a #define > with a check for the above would be needed. That may be a good idea, but please notice that this check would not be there with just an array or a pointer. FFmpeg is C, not Java. We do not HAVE to add the check. > And since both are used as frame side data, it would be ideal that the > signature for the public helpers on both are the same (The standard alloc, > and the alloc + wrapping into frame side data ones). I agree with this. 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 V6 4/6] lavu: add side data AV_FRAME_DATA_BOUNDING_BOXES
Andreas Rheinhardt (12021-04-04): > That's a variable-length array. That's a C99 feature we do not require. We have used variable-length arrays in the past. We have eliminated them from our code base not because lack of support, IIRC, but because they have a tendency to ruin compiler optimization. This is not a variable-length array but a pointer to a variable-length array, it does not cause the optimization problems. As it is, it is just a trick to let the compiler compute offsets for us taking into account alignment requirements. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/3] avformat/rtsp: Reindent after previous commit
From: Andriy Gelman
Signed-off-by: Andriy Gelman
---
libavformat/rtsp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 76efbf42cd..6438e2edb8 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1466,9 +1466,9 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const
char *host, int port,
* port range, to allow for a number of ports to try even if the offset
* happens to be at the end of the random range. */
if (rt->rtp_port_max - rt->rtp_port_min > 1) {
-port_off = av_get_random_seed() % ((rt->rtp_port_max -
rt->rtp_port_min)/2);
-/* even random offset */
-port_off -= port_off & 0x01;
+port_off = av_get_random_seed() % ((rt->rtp_port_max -
rt->rtp_port_min)/2);
+/* even random offset */
+port_off -= port_off & 0x01;
}
for (j = rt->rtp_port_min + port_off, i = 0; i < rt->nb_rtsp_streams; ++i)
{
--
2.31.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/3] avformat/rtsp: Include rtcp in port range check
From: Andriy Gelman
Currently it is only checked that the rtp port does not exceed rtp_port_max.
---
libavformat/rtsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 6438e2edb8..e6a068148b 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1508,7 +1508,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const
char *host, int port,
}
/* first try in specified port range */
-while (j <= rt->rtp_port_max) {
+while (j + 1 <= rt->rtp_port_max) {
AVDictionary *opts = map_to_opts(rt);
ff_url_join(buf, sizeof(buf), "rtp", NULL, host, -1,
--
2.31.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/3] avformat/rtsp: Fix floating point exception for low min/max port range
From: Andriy Gelman
Fixed by setting port offset to zero when it cannot be computed.
To reproduce:
$ ffmpeg -min_port 32000 -max_port 32001 -i
rtsp://wowzaec2demo.streamlock.net/vod/mp4:BigBuckBunny_115k.mov -f null -
[1]303871 floating point exception (core dumped)
Signed-off-by: Andriy Gelman
---
libavformat/rtsp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 25bdf475b3..76efbf42cd 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1446,7 +1446,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const
char *host, int port,
int lower_transport, const char *real_challenge)
{
RTSPState *rt = s->priv_data;
-int rtx = 0, j, i, err, interleave = 0, port_off;
+int rtx = 0, j, i, err, interleave = 0, port_off = 0;
RTSPStream *rtsp_st;
RTSPMessageHeader reply1, *reply = &reply1;
char cmd[MAX_URL_SIZE];
@@ -1465,9 +1465,11 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const
char *host, int port,
/* Choose a random starting offset within the first half of the
* port range, to allow for a number of ports to try even if the offset
* happens to be at the end of the random range. */
+if (rt->rtp_port_max - rt->rtp_port_min > 1) {
port_off = av_get_random_seed() % ((rt->rtp_port_max -
rt->rtp_port_min)/2);
/* even random offset */
port_off -= port_off & 0x01;
+}
for (j = rt->rtp_port_min + port_off, i = 0; i < rt->nb_rtsp_streams; ++i)
{
char transport[MAX_URL_SIZE];
--
2.31.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] avformat/mpegtsenc: Preserve disposition in the absence of language
Marton Balint:
>
>
> On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:
>
>> Implements ticket #9113.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavformat/mpegtsenc.c | 18 +-
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
>> index 35c835c484..dbd3bb148a 100644
>> --- a/libavformat/mpegtsenc.c
>> +++ b/libavformat/mpegtsenc.c
>> @@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s,
>> MpegTSService *service)
>> AVStream *st = s->streams[i];
>> MpegTSWriteStream *ts_st = st->priv_data;
>> AVDictionaryEntry *lang = av_dict_get(st->metadata,
>> "language", NULL, 0);
>> + const char default_language[] = "und";
>> + const char *language = lang && strlen(lang->value) >= 3 ?
>> lang->value : default_language;
>> enum AVCodecID codec_id = st->codecpar->codec_id;
>>
>> if (s->nb_programs) {
>> @@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s,
>> MpegTSService *service)
>> }
>> }
>>
>> - if (lang) {
>> - char *p;
>> - char *next = lang->value;
>> + if (language != default_language ||
>> + st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS |
>> + AV_DISPOSITION_HEARING_IMPAIRED |
>> + AV_DISPOSITION_VISUAL_IMPAIRED)) {
>> + const char *p;
>> + const char *next = language;
>> uint8_t *len_ptr;
>>
>> *q++ = ISO_639_LANGUAGE_DESCRIPTOR;
>> len_ptr = q++;
>> *len_ptr = 0;
>>
>> - for (p = lang->value; next && *len_ptr < 255 / 4 * 4;
>> p = next + 1) {
>> + for (p = next; next && *len_ptr < 255 / 4 * 4; p =
>> next + 1) {
>
> Maybe it would make the code more readable to do both initializations in
> the for() statement, e.g.: for (p = next = language; ...)
>
The reason I didn't do so is that it would make the line above 80
characters. So the alternatives to my current patch would be:
for (p = next = language; next && *len_ptr < 255 / 4 * 4;
p = next + 1) {
or
for (const char *p = language, *next = p;
next && *len_ptr < 255 / 4 * 4; p = next + 1) {
or factoring out writing the ISO 639 language descriptor into a function
of its own or just ignoring the 80 chars line limit. What do you prefer?
> LGTM otherwise.
>
> Thanks,
> Marton
>
>> if (q - data > SECTION_LENGTH - 4) {
>> err = 1;
>> break;
>> @@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
>> MpegTSService *service)
>> }
>> break;
>> case AVMEDIA_TYPE_SUBTITLE:
>> - {
>> - const char default_language[] = "und";
>> - const char *language = lang && strlen(lang->value) >= 3 ?
>> lang->value : default_language;
>> -
>> if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
>> uint8_t *len_ptr;
>> int extradata_copied = 0;
>> @@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
>> MpegTSService *service)
>>
>> *len_ptr = q - len_ptr - 1;
>> }
>> - }
>> break;
>> case AVMEDIA_TYPE_VIDEO:
>> if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
>> --
>> 2.27.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 mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/mpegtsenc: Preserve disposition in the absence of language
On Sun, 4 Apr 2021, Andreas Rheinhardt wrote:
Marton Balint:
On Sat, 3 Apr 2021, Andreas Rheinhardt wrote:
Implements ticket #9113.
Signed-off-by: Andreas Rheinhardt
---
libavformat/mpegtsenc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
index 35c835c484..dbd3bb148a 100644
--- a/libavformat/mpegtsenc.c
+++ b/libavformat/mpegtsenc.c
@@ -459,6 +459,8 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
AVStream *st = s->streams[i];
MpegTSWriteStream *ts_st = st->priv_data;
AVDictionaryEntry *lang = av_dict_get(st->metadata,
"language", NULL, 0);
+ const char default_language[] = "und";
+ const char *language = lang && strlen(lang->value) >= 3 ?
lang->value : default_language;
enum AVCodecID codec_id = st->codecpar->codec_id;
if (s->nb_programs) {
@@ -598,16 +600,19 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
}
}
- if (lang) {
- char *p;
- char *next = lang->value;
+ if (language != default_language ||
+ st->disposition & (AV_DISPOSITION_CLEAN_EFFECTS |
+ AV_DISPOSITION_HEARING_IMPAIRED |
+ AV_DISPOSITION_VISUAL_IMPAIRED)) {
+ const char *p;
+ const char *next = language;
uint8_t *len_ptr;
*q++ = ISO_639_LANGUAGE_DESCRIPTOR;
len_ptr = q++;
*len_ptr = 0;
- for (p = lang->value; next && *len_ptr < 255 / 4 * 4;
p = next + 1) {
+ for (p = next; next && *len_ptr < 255 / 4 * 4; p =
next + 1) {
Maybe it would make the code more readable to do both initializations in
the for() statement, e.g.: for (p = next = language; ...)
The reason I didn't do so is that it would make the line above 80
characters. So the alternatives to my current patch would be:
for (p = next = language; next && *len_ptr < 255 / 4 * 4;
p = next + 1) {
I like this the most wrapped to a single line. I don't think adhereing to
the 80 char limit makes the code more readable here, so I'd simply ignore
it.
Regards,
Marton
or
for (const char *p = language, *next = p;
next && *len_ptr < 255 / 4 * 4; p = next + 1) {
or factoring out writing the ISO 639 language descriptor into a function
of its own or just ignoring the 80 chars line limit.
What do you prefer?
LGTM otherwise.
Thanks,
Marton
if (q - data > SECTION_LENGTH - 4) {
err = 1;
break;
@@ -637,10 +642,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
}
break;
case AVMEDIA_TYPE_SUBTITLE:
- {
- const char default_language[] = "und";
- const char *language = lang && strlen(lang->value) >= 3 ?
lang->value : default_language;
-
if (codec_id == AV_CODEC_ID_DVB_SUBTITLE) {
uint8_t *len_ptr;
int extradata_copied = 0;
@@ -715,7 +716,6 @@ static int mpegts_write_pmt(AVFormatContext *s,
MpegTSService *service)
*len_ptr = q - len_ptr - 1;
}
- }
break;
case AVMEDIA_TYPE_VIDEO:
if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
--
2.27.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 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] avutil/video_enc_params: schedule the size overflow check for removal
av_buffer_create() will start taking a size argument of size_t type.
Signed-off-by: James Almer
---
libavutil/video_enc_params.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c
index 635176ab91..e6a8e38d94 100644
--- a/libavutil/video_enc_params.c
+++ b/libavutil/video_enc_params.c
@@ -63,10 +63,12 @@ av_video_enc_params_create_side_data(AVFrame *frame, enum
AVVideoEncParamsType t
par = av_video_enc_params_alloc(type, nb_blocks, &size);
if (!par)
return NULL;
+#if FF_API_BUFFER_SIZE_T
if (size > INT_MAX) {
av_free(par);
return NULL;
}
+#endif
buf = av_buffer_create((uint8_t *)par, size, NULL, NULL, 0);
if (!buf) {
av_freep(&par);
--
2.31.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] [RFC] CFHD
On 4/4/2021 4:42 AM, Paul B Mahol wrote: On Sat, Apr 3, 2021 at 1:01 PM Anton Khirnov wrote: Quoting Michael Niedermayer (2021-04-02 20:27:24) Hi all CFHD currently has even with all fixes (ignoring ones with objections) applied a null pointer read and out of array write issue remaining. My patch which makes the header parsing more restrictive has an objection against it. and the only other developer who recently worked on it stated that he has no "time or motivation to deal with this and similar issues" IMO any objection to a patch that does not include a clearly spelled out reason for the objection and/or an alternative solution should be ignored or regarded as spamming the ML. Especially when the patch addresses crashes or other security issues. Nice, You also prefer non-really security fixes. Good job team. I'm not going to repeat what I said about original patch(es). Could you at least suggest what would be a proper fix? The code can't be left like this, so something needs to be done. You disliked Michael's approach, so what's the alternative? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/3] avcodec/cfhd: Check transform_type consistently
On 4/4/2021 4:46 AM, Paul B Mahol wrote: As always, make sure that you do not break or add new regressions for decoding existing valid files. Coverage of cfhd by FATE is really low. Pretty much half of the decoder is untested right now: http://coverage.ffmpeg.org/index.src_libavcodec_cfhd.c.html Are there any samples with no copyright/license issues we could cut and add that make use of these untested code paths? On Sat, Apr 3, 2021 at 4:54 PM Michael Niedermayer wrote: On Sat, Apr 03, 2021 at 04:39:06PM +0200, Michael Niedermayer wrote: Fixes: out of array accesses Fixes: 29754/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6333598414274560 Fixes: 30519/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-6298424511168512 Fixes: 30739/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CFHD_fuzzer-5011292836462592 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer --- libavcodec/cfhd.c | 11 +-- libavcodec/cfhd.h | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) I intend to apply this patchset soon. Also this patchset almost certainly does not fix every issue in CFHD, so if someone is searching for code to do a security review on. CFHD is likely an interresting candidate thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB it is not once nor twice but times without number that the same ideas make their appearance in the world. -- Aristotle ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/mov: check offset for overflow in mov_probe()
Fixes: Invalid read of size 4
Fixes: ASAN_Deadlysignal.zip
Found-by: Hardik Shah
Signed-off-by: Michael Niedermayer
---
libavformat/mov.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 7805330bf9..ef73f3199d 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7161,6 +7161,8 @@ static int mov_probe(const AVProbeData *p)
score = FFMAX(score, AVPROBE_SCORE_EXTENSION);
break;
}
+if ((uint64_t)size + 8 > INT64_MAX - offset)
+break;
offset += size;
}
if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {
--
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] avformat/mov: check offset for overflow in mov_probe()
On 4/4/2021 6:44 PM, Michael Niedermayer wrote:
Fixes: Invalid read of size 4
Fixes: ASAN_Deadlysignal.zip
Found-by: Hardik Shah
Signed-off-by: Michael Niedermayer
---
libavformat/mov.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 7805330bf9..ef73f3199d 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7161,6 +7161,8 @@ static int mov_probe(const AVProbeData *p)
score = FFMAX(score, AVPROBE_SCORE_EXTENSION);
break;
}
+if ((uint64_t)size + 8 > INT64_MAX - offset)
+break;
offset += size;
}
if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {
Would something like this also work?
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 1974498d1e..cd9d9996b3 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -7119,7 +7119,7 @@ static int mov_probe(const AVProbeData *p)
int64_t size;
int minsize = 8;
/* ignore invalid offset */
-if ((offset + 8) > (unsigned int)p->buf_size)
+if ((offset + 8ULL) > (unsigned int)p->buf_size)
break;
size = AV_RB32(p->buf + offset);
if (size == 1 && offset + 16 <= (unsigned int)p->buf_size) {
@@ -7166,6 +7166,8 @@ static int mov_probe(const AVProbeData *p)
score = FFMAX(score, AVPROBE_SCORE_EXTENSION);
break;
}
+if (size > INT64_MAX - offset)
+break;
offset += size;
}
if (score > AVPROBE_SCORE_MAX - 50 && moov_offset != -1) {
I think it conveys what it's trying to do more clearly than your
version, where the + 8 on top of size is not immediately clear where it
comes from.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/4] avformat/rtsp: set AV_OPT_FLAG_DEPRECATED on depracated options
On Sun, 10. Jan 10:57, Andriy Gelman wrote:
> On Thu, 17. Dec 10:42, Andriy Gelman wrote:
> > On Tue, 08. Dec 22:35, Andriy Gelman wrote:
> > > Hi Zhao,
> > >
> > > Thanks for reviewing.
> > >
> > > On Tue, 08. Dec 13:25, "zhilizhao(赵志立)" wrote:
> > > >
> > > >
> > > > > On Dec 8, 2020, at 12:08 PM, Andriy Gelman
> > > > > wrote:
> > > > >
> > > > > On Sun, 15. Nov 13:20, Andriy Gelman wrote:
> > > > >> From: Andriy Gelman
> > > > >>
> > > > >> Signed-off-by: Andriy Gelman
> > > > >> ---
> > > > >> libavformat/rtsp.c | 4 ++--
> > > > >> 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >>
> > > > >> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > > > >> index d9832bbf1f..2ef75f50e3 100644
> > > > >> --- a/libavformat/rtsp.c
> > > > >> +++ b/libavformat/rtsp.c
> > > > >> @@ -94,7 +94,7 @@ const AVOption ff_rtsp_options[] = {
> > > > >> { "max_port", "set maximum local UDP port",
> > > > >> OFFSET(rtp_port_max), AV_OPT_TYPE_INT, {.i64 = RTSP_RTP_PORT_MAX},
> > > > >> 0, 65535, DEC|ENC },
> > > > >> { "listen_timeout", "set maximum timeout (in seconds) to wait
> > > > >> for incoming connections (-1 is infinite, imply flag listen)",
> > > > >> OFFSET(initial_timeout), AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN,
> > > > >> INT_MAX, DEC },
> > > > >> #if FF_API_OLD_RTSP_OPTIONS
> > > > >> -{ "timeout", "set maximum timeout (in seconds) to wait for
> > > > >> incoming connections (-1 is infinite, imply flag listen)
> > > > >> (deprecated, use listen_timeout)", OFFSET(initial_timeout),
> > > > >> AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX, DEC },
> > > > >> +{ "timeout", "set maximum timeout (in seconds) to wait for
> > > > >> incoming connections (-1 is infinite, imply flag listen)
> > > > >> (deprecated, use listen_timeout)", OFFSET(initial_timeout),
> > > > >> AV_OPT_TYPE_INT, {.i64 = -1}, INT_MIN, INT_MAX,
> > > > >> DEC|AV_OPT_FLAG_DEPRECATED },
> > > > >> { "stimeout", "set timeout (in microseconds) of socket TCP I/O
> > > > >> operations", OFFSET(stimeout), AV_OPT_TYPE_INT, {.i64 = 0}, INT_MIN,
> > > > >> INT_MAX, DEC },
> > > >
> > > > Looks good to me, although it’s a little weird that after major bump
> > > > “timeout”
> > > > will have a different meaning instead of being dropped. “stimeout” is
> > > > deprecated, since there is not other option to replace it at the
> > > > current time,
> > > > it cannot be marked as AV_OPT_FLAG_DEPRECATED.
> > > >
> > >
> > > Right, after the major bump timeout will become the suggested alternative
> > > for
> > > stimeout, and stimeout will have the deprecated label.
> > > I think the idea is to get away from timeout option implying the listen
> > > mode.
> >
> > Will apply this patch.
> >
> > Ping for patches 2-4.
> >
>
> ping
>
ping
Will apply 2-4 later in the week if no one objects.
--
Andriy
___
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/6] Revert "avcodec: add FF_CODEC_CAP_INIT_CLEANUP for all codecs which use ff_mpv_common_init()"
From: Andreas Rheinhardt
This mostly reverts commit 4b2863ff01b1fe93d9a518523c9098d17a9d8c6f.
Said commit removed the freeing code from ff_mpv_common_init(),
ff_mpv_common_frame_size_change() and ff_mpeg_framesize_alloc() and
instead added the FF_CODEC_CAP_INIT_CLEANUP to several codecs that use
ff_mpv_common_init(). This introduced several bugs:
a) Several decoders using ff_mpv_common_init() in their init function were
forgotten: This affected FLV, Intel H.263, RealVideo 3.0 and V4.0 as well as
VC-1/WMV3.
b) ff_mpv_common_init() is not only called from the init function of
codecs, it is also called from AVCodec.decode functions. If an error
happens after an allocation has succeeded, it can lead to memleaks;
furthermore, it is now possible for the MpegEncContext to be marked as
initialized even when ff_mpv_common_init() returns an error and this can
lead to segfaults because decoders that call ff_mpv_common_init() when
decoding a frame can mistakenly think that the MpegEncContext has been
properly initialized. This can e.g. happen with H.261 or MPEG-4.
c) Removing code for freeing from ff_mpeg_framesize_alloc() (which can't
be called from any init function) can lead to segfaults because the
check for whether it needs to allocate consists of checking whether the
first of the buffers allocated there has been allocated. This part has
already been fixed in 76cea1d2ce3f23e8131c8664086a1daf873ed694.
d) ff_mpv_common_frame_size_change() can also not be reached from any
AVCodec.init function; yet the changes can e.g. lead to segfaults with
decoders using ff_h263_decode_frame() upon allocation failure, because
the MpegEncContext will upon return be flagged as both initialized and
not in need of reinitialization (granted, the fact that
ff_h263_decode_frame() clears context_reinit before the context has been
reinited is a bug in itself). With the earlier version, the context
would be cleaned upon failure and it would be attempted to initialize
the context again in the next call to ff_h263_decode_frame().
While a) could be fixed by adding the missing FF_CODEC_CAP_INIT_CLEANUP,
keeping the current approach would entail adding cleanup code to several
other places because of b). Therefore ff_mpv_common_init() is again made
to clean up after itself; the changes to the wmv2 decoder and the SVQ1
encoder have not been reverted: The former fixed a memleak, the latter
allowed to remove cleanup code.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/h261dec.c | 2 +-
libavcodec/h263dec.c | 4 ++--
libavcodec/mpeg12dec.c | 9 +
libavcodec/mpeg4videodec.c | 3 +--
libavcodec/mpegvideo.c | 31 ---
libavcodec/msmpeg4dec.c| 8
libavcodec/rv10.c | 2 --
7 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c
index 5c25aa9cb3..eb544e6046 100644
--- a/libavcodec/h261dec.c
+++ b/libavcodec/h261dec.c
@@ -678,6 +678,6 @@ AVCodec ff_h261_decoder = {
.close = h261_decode_end,
.decode = h261_decode_frame,
.capabilities = AV_CODEC_CAP_DR1,
-.caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
+.caps_internal = FF_CODEC_CAP_INIT_THREADSAFE,
.max_lowres = 3,
};
diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index dafa54d8d4..e8b4d83e6e 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -771,7 +771,7 @@ AVCodec ff_h263_decoder = {
.decode = ff_h263_decode_frame,
.capabilities = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY,
-.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
FF_CODEC_CAP_INIT_CLEANUP,
+.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
.flush = ff_mpeg_flush,
.max_lowres = 3,
.pix_fmts = ff_h263_hwaccel_pixfmt_list_420,
@@ -789,7 +789,7 @@ AVCodec ff_h263p_decoder = {
.decode = ff_h263_decode_frame,
.capabilities = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_TRUNCATED | AV_CODEC_CAP_DELAY,
-.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
FF_CODEC_CAP_INIT_CLEANUP,
+.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
.flush = ff_mpeg_flush,
.max_lowres = 3,
.pix_fmts = ff_h263_hwaccel_pixfmt_list_420,
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 92dd6a0b24..94221da2c1 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2880,7 +2880,8 @@ static av_cold int mpeg_decode_end(AVCodecContext *avctx)
{
Mpeg1Context *s = avctx->priv_data;
-ff_mpv_common_end(&s->mpeg_enc_ctx);
+if (s->mpeg_enc_ctx_allocated)
+ff_mpv_common_end(&s->mpeg_enc_ctx);
av_buffer_unref(&s->a53_buf_ref);
return 0;
}
@@ -2897,7 +2898,7 @@ AVCodec ff_mpeg1video_decoder = {
.capabilities = AV_
[FFmpeg-devel] [PATCH 2/6] avcodec/mpegvideo: Fix memleak upon allocation error
From: Andreas Rheinhardt
When slice-threading is used, ff_mpv_common_init() duplicates
the first MpegEncContext and allocates some buffers for each
MpegEncContext (the first as well as the copies). But the count of
allocated MpegEncContexts is not updated until after everything has
been allocated and if an error happens after the first one has been
allocated, only the first one is freed; the others leak.
This commit fixes this: The count is now set before the copies are
allocated. Furthermore, the copies are now created and initialized
before the first MpegEncContext, so that the buffers exclusively owned
by each MpegEncContext are still NULL in the src MpegEncContext so
that no double-free happens upon allocation failure.
Given that this effectively touches every line of the init code,
it has also been factored out in a function of its own in order to
remove code duplication with the same code in
ff_mpv_common_frame_size_change() (which was never called when using
more than one slice (and if it were, there would be potential
double-frees)).
Signed-off-by: Andreas Rheinhardt
---
libavcodec/mpegvideo.c | 91 +-
1 file changed, 37 insertions(+), 54 deletions(-)
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 2351a09874..7327204e99 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -366,13 +366,6 @@ static int init_duplicate_context(MpegEncContext *s)
if (s->mb_height & 1)
yc_size += 2*s->b8_stride + 2*s->mb_stride;
-s->sc.edge_emu_buffer =
-s->me.scratchpad =
-s->me.temp =
-s->sc.rd_scratchpad =
-s->sc.b_scratchpad=
-s->sc.obmc_scratchpad = NULL;
-
if (s->encoding) {
if (!FF_ALLOCZ_TYPED_ARRAY(s->me.map, ME_MAP_SIZE) ||
!FF_ALLOCZ_TYPED_ARRAY(s->me.score_map, ME_MAP_SIZE))
@@ -413,6 +406,35 @@ static int init_duplicate_context(MpegEncContext *s)
return 0;
}
+/**
+ * Initialize an MpegEncContext's thread contexts. Presumes that
+ * slice_context_count is already set and that all the fields
+ * that are freed/reset in free_duplicate_context() are NULL.
+ */
+static int init_duplicate_contexts(MpegEncContext *s)
+{
+int nb_slices = s->slice_context_count, ret;
+
+/* We initialize the copies before the original so that
+ * fields allocated in init_duplicate_context are NULL after
+ * copying. This prevents double-frees upon allocation error. */
+for (int i = 1; i < nb_slices; i++) {
+s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext));
+if (!s->thread_context[i])
+return AVERROR(ENOMEM);
+if ((ret = init_duplicate_context(s->thread_context[i])) < 0)
+return ret;
+s->thread_context[i]->start_mb_y =
+(s->mb_height * (i) + nb_slices / 2) / nb_slices;
+s->thread_context[i]->end_mb_y =
+(s->mb_height * (i + 1) + nb_slices / 2) / nb_slices;
+}
+s->start_mb_y = 0;
+s->end_mb_y = nb_slices > 1 ? (s->mb_height + nb_slices / 2) / nb_slices
+ : s->mb_height;
+return init_duplicate_context(s);
+}
+
static void free_duplicate_context(MpegEncContext *s)
{
if (!s)
@@ -949,29 +971,12 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
s->context_initialized = 1;
memset(s->thread_context, 0, sizeof(s->thread_context));
s->thread_context[0] = s;
+s->slice_context_count = nb_slices;
// if (s->width && s->height) {
-if (nb_slices > 1) {
-for (i = 0; i < nb_slices; i++) {
-if (i) {
-s->thread_context[i] = av_memdup(s, sizeof(MpegEncContext));
-if (!s->thread_context[i])
-goto fail_nomem;
-}
-if ((ret = init_duplicate_context(s->thread_context[i])) < 0)
-goto fail;
-s->thread_context[i]->start_mb_y =
-(s->mb_height * (i) + nb_slices / 2) / nb_slices;
-s->thread_context[i]->end_mb_y =
-(s->mb_height * (i + 1) + nb_slices / 2) / nb_slices;
-}
-} else {
-if ((ret = init_duplicate_context(s)) < 0)
-goto fail;
-s->start_mb_y = 0;
-s->end_mb_y = s->mb_height;
-}
-s->slice_context_count = nb_slices;
+ret = init_duplicate_contexts(s);
+if (ret < 0)
+goto fail;
// }
return 0;
@@ -1079,7 +1084,7 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
&s->chroma_x_shift,
&s->chroma_y_shift);
if (err < 0)
-return err;
+goto fail;
if ((err = init_context_frame(s)))
goto fail;
@@ -1088,31 +1093,9 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
s->thread_context[0] = s;
if (s->width && s->height) {
-int nb_slices = s->slice_context_count;
-if (nb_
[FFmpeg-devel] [PATCH 3/6] avcodec/mpegvideo: Factor common freeing code out
From: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt
---
libavcodec/mpegvideo.c | 36 +++-
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 7327204e99..7eddbdcc37 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -457,6 +457,15 @@ static void free_duplicate_context(MpegEncContext *s)
s->block = NULL;
}
+static void free_duplicate_contexts(MpegEncContext *s)
+{
+for (int i = 1; i < s->slice_context_count; i++) {
+free_duplicate_context(s->thread_context[i]);
+av_freep(&s->thread_context[i]);
+}
+free_duplicate_context(s);
+}
+
static void backup_duplicate_context(MpegEncContext *bak, MpegEncContext *src)
{
#define COPY(a) bak->a = src->a
@@ -988,7 +997,8 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
}
/**
- * Frees and resets MpegEncContext fields depending on the resolution.
+ * Frees and resets MpegEncContext fields depending on the resolution
+ * as well as the slice thread contexts.
* Is used during resolution changes to avoid a full reinitialization of the
* codec.
*/
@@ -996,6 +1006,8 @@ static void free_context_frame(MpegEncContext *s)
{
int i, j, k;
+free_duplicate_contexts(s);
+
av_freep(&s->mb_type);
av_freep(&s->p_mv_table_base);
av_freep(&s->b_forw_mv_table_base);
@@ -1048,16 +1060,6 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
if (!s->context_initialized)
return AVERROR(EINVAL);
-if (s->slice_context_count > 1) {
-for (i = 0; i < s->slice_context_count; i++) {
-free_duplicate_context(s->thread_context[i]);
-}
-for (i = 1; i < s->slice_context_count; i++) {
-av_freep(&s->thread_context[i]);
-}
-} else
-free_duplicate_context(s);
-
free_context_frame(s);
if (s->picture)
@@ -1112,15 +1114,9 @@ void ff_mpv_common_end(MpegEncContext *s)
if (!s)
return;
-if (s->slice_context_count > 1) {
-for (i = 0; i < s->slice_context_count; i++) {
-free_duplicate_context(s->thread_context[i]);
-}
-for (i = 1; i < s->slice_context_count; i++) {
-av_freep(&s->thread_context[i]);
-}
+free_context_frame(s);
+if (s->slice_context_count > 1)
s->slice_context_count = 1;
-} else free_duplicate_context(s);
av_freep(&s->parse_context.buffer);
s->parse_context.buffer_size = 0;
@@ -1152,8 +1148,6 @@ void ff_mpv_common_end(MpegEncContext *s)
ff_mpeg_unref_picture(s->avctx, &s->new_picture);
av_frame_free(&s->new_picture.f);
-free_context_frame(s);
-
s->context_initialized = 0;
s->last_picture_ptr =
s->next_picture_ptr =
--
2.27.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 4/6] avcodec/rv10: Don't presume context to be initialized
In case of resolution changes rv20_decode_picture_header() closes and
reopens its MpegEncContext; it checks the latter for errors, yet when
an error happens, it might happen that no new attempt at
reinitialization is performed when decoding the next frame; this leads
to crashes lateron.
This commit changes this by making sure that initialization will always
be attempted if the context is currently not initialized.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/rv10.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/rv10.c b/libavcodec/rv10.c
index 89b838ad15..bd70689cab 100644
--- a/libavcodec/rv10.c
+++ b/libavcodec/rv10.c
@@ -226,7 +226,7 @@ static int rv20_decode_picture_header(RVDecContext *rv)
new_w = rv->orig_width;
new_h = rv->orig_height;
}
-if (new_w != s->width || new_h != s->height) {
+if (new_w != s->width || new_h != s->height ||
!s->context_initialized) {
AVRational old_aspect = s->avctx->sample_aspect_ratio;
av_log(s->avctx, AV_LOG_DEBUG,
"attempting to change resolution to %dx%d\n", new_w, new_h);
--
2.27.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 5/6] avcodec/rv34, mpegvideo: Fix segfault upon frame size change error
The RealVideo 3.0 and 4.0 decoders call ff_mpv_common_init() only during
their init function and not during decode_frame(); when the size of the
frame changes, they call ff_mpv_common_frame_size_change(). Yet upon
error, said function calls ff_mpv_common_end() which frees the whole
MpegEncContext and not only those parts that
ff_mpv_common_frame_size_change() reinits. As a result, the context will
never be usable again; worse, because decode_frame() contains no check
for whether the context is initialized or not, it is presumed that it is
initialized, leading to segfaults. Basically the same happens if
rv34_decoder_realloc() fails.
This commit fixes this by only resetting the parts that
ff_mpv_common_frame_size_change() changes upon error and by actually
checking whether the context is in need of reinitialization in
ff_rv34_decode_frame().
Signed-off-by: Andreas Rheinhardt
---
I actually don't like that we have two flags that indicate whether
a MpegEncContext is usable or not; how about we always call
ff_mpv_common_init() during init (and never lateron) and make it
unconditionally allocate the stuff that does not depend upon resolution
etc. and add a parameter to said function to also allocate the latter.
The decode_frame functions would then be modified to always use
ff_mpv_common_frame_size_change().
ff_rv34_decode_update_thread_context() currently checks twice whether
the source MpegEncContext is initialized; the second is trivially true,
because of the first check. But the first is also always true now,
because ff_mpv_common_end() is only called when closing the decoder
(and said check also made no sense before this patch because a
noninitialized MpegEncContext would have led to a segfault pretty soon).
Should this be changed to check for context_reinit instead?
libavcodec/mpegvideo.c | 6 --
libavcodec/rv34.c | 13 +
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 7eddbdcc37..5de0719f83 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -555,7 +555,6 @@ int ff_mpeg_update_thread_context(AVCodecContext *dst,
}
if (s->height != s1->height || s->width != s1->width || s->context_reinit)
{
-s->context_reinit = 0;
s->height = s1->height;
s->width = s1->width;
if ((ret = ff_mpv_common_frame_size_change(s)) < 0)
@@ -1099,10 +1098,12 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
if (err < 0)
goto fail;
}
+s->context_reinit = 0;
return 0;
fail:
-ff_mpv_common_end(s);
+free_context_frame(s);
+s->context_reinit = 1;
return err;
}
@@ -1149,6 +1150,7 @@ void ff_mpv_common_end(MpegEncContext *s)
av_frame_free(&s->new_picture.f);
s->context_initialized = 0;
+s->context_reinit = 0;
s->last_picture_ptr =
s->next_picture_ptr =
s->current_picture_ptr = NULL;
diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
index 7e5bfe0e22..99e580a09a 100644
--- a/libavcodec/rv34.c
+++ b/libavcodec/rv34.c
@@ -1383,6 +1383,7 @@ static int rv34_decoder_alloc(RV34DecContext *r)
if (!(r->cbp_chroma && r->cbp_luma && r->deblock_coefs &&
r->intra_types_hist && r->mb_type)) {
+r->s.context_reinit = 1;
rv34_decoder_free(r);
return AVERROR(ENOMEM);
}
@@ -1530,7 +1531,7 @@ int ff_rv34_decode_update_thread_context(AVCodecContext
*dst, const AVCodecConte
if (dst == src || !s1->context_initialized)
return 0;
-if (s->height != s1->height || s->width != s1->width) {
+if (s->height != s1->height || s->width != s1->width || s->context_reinit)
{
s->height = s1->height;
s->width = s1->width;
if ((err = ff_mpv_common_frame_size_change(s)) < 0)
@@ -1667,11 +1668,12 @@ int ff_rv34_decode_frame(AVCodecContext *avctx,
if (s->mb_num_left > 0 && s->current_picture_ptr) {
av_log(avctx, AV_LOG_ERROR, "New frame but still %d MB left.\n",
s->mb_num_left);
-ff_er_frame_end(&s->er);
+if (!s->context_reinit)
+ff_er_frame_end(&s->er);
ff_mpv_frame_end(s);
}
-if (s->width != si.width || s->height != si.height) {
+if (s->width != si.width || s->height != si.height ||
s->context_reinit) {
int err;
av_log(s->avctx, AV_LOG_WARNING, "Changing dimensions to %dx%d\n",
@@ -1689,7 +1691,6 @@ int ff_rv34_decode_frame(AVCodecContext *avctx,
err = ff_set_dimensions(s->avctx, s->width, s->height);
if (err < 0)
return err;
-
if ((err = ff_mpv_common_frame_size_change(s)) < 0)
return err;
if ((err = rv34_decoder_realloc(r)) < 0)
@@ -1744,6 +1745,10 @@ int ff_rv34_decode_frame(AVCodecContext *avctx,
}
s->mb_x = s->mb_y = 0;
[FFmpeg-devel] [PATCH 6/6] avcodec/h261dec: Initialize IDCT context during init
Before 998c9f15d1ca8c7489775ebcca51623b915988f1, initializing an MpegEncContext's IDCT parts occured in ff_mpv_common_init() and this has been called in h261_decode_frame(), not h261_decode_init(). Yet said commit factored this out of ff_mpv_common_init() and therefore there is no reason any more not to set this during init as this commit does. Signed-off-by: Andreas Rheinhardt --- libavcodec/h261dec.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libavcodec/h261dec.c b/libavcodec/h261dec.c index eb544e6046..0d8cd8c20d 100644 --- a/libavcodec/h261dec.c +++ b/libavcodec/h261dec.c @@ -79,6 +79,7 @@ static av_cold int h261_decode_init(AVCodecContext *avctx) avctx->pix_fmt = AV_PIX_FMT_YUV420P; h->gob_start_code_skipped = 0; +ff_mpv_idct_init(s); ff_thread_once(&init_static_once, h261_decode_init_static); @@ -595,10 +596,6 @@ static int h261_decode_frame(AVCodecContext *avctx, void *data, retry: init_get_bits(&s->gb, buf, buf_size * 8); -if (!s->context_initialized) -// we need the IDCT permutation for reading a custom matrix -ff_mpv_idct_init(s); - ret = h261_decode_picture_header(h); /* skip if the header was thrashed */ -- 2.27.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".
