Re: [FFmpeg-devel] [PATCH] lavfi: get rid of FF_INTERNAL_FIELDS
On Fri, 2023-02-03 at 15:45 +0100, Nicolas George wrote:
> Andreas Rheinhardt (12023-02-01):
> > "One special guarantee is made in order to simplify the use of unions:
> > if a union contains several structures that share a common initial
> > sequence (see below), and if the union object currently contains one of
> > these structures, it is permitted to inspect the common initial part of
> > any of them anywhere that a declaration of the completed type of the
> > union is visible. Two structures share a common initial sequence if
> > corresponding members have compatible types (and, for bit-fields, the
> > same widths) for a sequence of one or more initial members."
> >
> > But there just is no union between the FF_INTERNAL_FIELDS
> > !defined(FF_INTERNAL_FIELDS) structures in the whole codebase.
>
> It is not necessary that there is one: it is enough that there could be
> one in another compilation unit, the code that handles these structures
> does not know what exists in the rest of the code base. This guarantee
> was made FOR unions, but it does not REQUIRE unions, it applies to
> anything that is semantically equivalent to a union.
This is not valid reasoning. Consider code such as
int f(struct s1 *p1, struct s2 *p2) {
p1->abc = 0;
p2->abc = 42;
return p1->abc;
}
where the field abc belongs to a shared initial sequence in s1 and s2.
If there is no common union visible when this code is compiled, it
seems valid for the compiler to hardcode this to return 0, even if it
may be called with p1 and p2 pointing to the same address. LTO may
generate the equivalent of this code when compiling a combination of
assignments and reads from files using different definitions for a
struct.
___
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: Rename RSHIFT macro to ROUNDED_RSHIFT
On Mon, 2019-01-28 at 00:04 +0100, Henrik Gramner wrote: > On Mon, Jan 21, 2019 at 9:54 PM James Almer wrote: > > There's also no good way to deprecate a define and replace it with > > another while informing the library user, so for something purely > > cosmetic like this i don't think it's worth the trouble. > > Would it be possible to create a deprecated inlined function that does > nothing, and add a call to that function inside the old macro? Kind of > ugly though. I already posted that suggestion last Tuesday. It does work trigger a deprecation warning, but an inline function has the drawback that it's not a constant expression. If you want to keep backward compatibility even for uses like initializing global tables or other variables, you can use a deprecated variable in ways other than a call though. For example this should be a constant expression that shows a deprecation message: // This variable does not need to really exist extern int __attribute__ ((deprecated)) RSHIFT_is_deprecated; #define RSHIFT(a, b) (0*(int)sizeof(RSHIFT_is_deprecated) + AV_ROUNDED_SHIFT(a, b)) sizeof counts as a "use" that shows the deprecation warning, while not creating any actual reference to the variable and being a constant expression. The (int) cast is there to make 100% sure that the addition doesn't change anything by changing the type of the expression. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Rename RSHIFT macro to ROUNDED_RSHIFT
On Tue, 2019-01-22 at 15:35 -0300, James Almer wrote:
> I'm not against renaming it to AV_ROUNDED_RSHIFT or similar, but other
> than an entry in APIChanges we have no way to let library users that
> RSHIFT will be removed two or so years from now.
How about:
static inline void __attribute__ ((deprecated)) RSHIFT_is_deprecated(void) {}
#define RSHIFT(a, b) (RSHIFT_is_deprecated(), AV_ROUNDED_SHIFT(a, b))
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] avcodec/htmlsubtitles: Protect very slow redundant sscanf() calls by optimized use of strchr()
On Sat, 2017-06-10 at 16:18 +0200, Michael Niedermayer wrote:
> So we are guranteed that anyone who wants to exploit this has the
> ability to do so as long as they can use the search mask and are able
> to remux the data into whatever format they need.
> And i belive this publication of issues is the right thing to do.
It looks like the current code has quadratic time requirements. Is
everything else in FFmpeg actually guaranteed to not need quadratic
time? Can anything really rely on FFmpeg decoding of hostile data not
taking a long time? To the degree that it would be reasonable to have a
system using FFmpeg on untrusted data without timeouts or capacity to
kill the process? To me being slow on malicious data doesn't seem like
a real security issue.
> Do you have a better idea than the next_closep code to fix this ?
> If not, do you agree that we push this fix ?
Since the slow behavior seems unlikely to occur except with
intentionally malicious or completely corrupted data, I'm not sure it's
worth actually fixing it. But I think it could be done somewhat more
cleanly as follows:
Instead of using scanf to find matches for "{Y:xxx}" or "{\xxx}", where
"xxx" is arbitrarily long, match for "{Y:" or "{\", and then do the
"skip until next }" as a separate step after you've confirmed a match.
Then you can optimize that to avoid quadratic behavior by setting a
flag when you find no "}", and not searching again if it's already set.
Also, the current code seems buggy, or at least I don't see why you'd
want it to behave like it now does. It skips "{\xxx}" tags when the
"an" variable is not set to 1. I assume the intent was to only allow
the first "\an" tag through. But as implemented now it seems to allow
ANY "{\xxx}" tags through after the first \an and before possible
second one. I don't think that's intentional?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] pthread_frame: don't return stale error codes after flush
On Thu, 2017-04-06 at 18:18 +0200, wm4 wrote: > > > p->got_frame = 0; > > > av_frame_unref(p->frame); > > > +p->result = 0; Shouldn't p->result be similarly reset together with p->got_frame also in ff_thread_decode_frame()? A similar problem seems possible: - a normal decode call returns an error due to p->result being negative - drain packet is sent before cycling through all threads - the loop in ff_thread_decode_frame hits "if (p->result < 0)" Thus incorrectly returning the same error again from the drain packet. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
