On 3/21/2021 3:37 PM, James Almer wrote:
On 3/21/2021 3:15 PM, Marton Balint wrote:
On Sun, 21 Mar 2021, James Almer wrote:
On 3/21/2021 11:16 AM, Anton Khirnov wrote:
Quoting James Almer (2021-03-21 14:54:22)
On 3/21/2021 9:28 AM, Anton Khirnov wrote:
Quoting James Almer (2021-03-05 17:32:52)
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 7da2f3d98e..783cc1b591 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -954,7 +954,11 @@ typedef struct AVStream {
* decoding: set by libavformat, must not be modified by the
caller.
* encoding: unused
*/
+#if FF_API_INIT_PACKET
AVPacket attached_pic;
+#else
+ AVPacket *attached_pic;
+#endif
Sorry I'm late to the party, but as we are changing the type of an
existing field, we need to explicitly spell out a way for the
callers to
make their code forward compatible. E.g. similarly to what I made for
thread_safe_callbacks deprecation.
What do you suggest? The field is not printing deprecation
warnings, so
would a doxy comment be enough for people to notice it?
Have we done a similar change before? I know at least for symbols like
public functions we never change their signature in an incompatible
way,
and instead replace them altogether.
Maybe we could add the deprecated attribute to attached_pic,
introduce a
temporary getter function that returns a pointer to the packet,
mention
in the doxy that the field is not going away, is just changing types,
and they have the option of using the getter for a fire-and-forget
migration process, then maybe deprecate and eventually remove the
getter
after the switch is done.
This seems like a lot of hoops to jump through. Wouldn't it then be
better to just add a new field with a new name?
A name change just because it's now a pointer seems overkill.
I don't think it is. A name change guarantees that existing code won't
compile to something that will surely crash. IMHO the clean solution
is to keep the old field with deprecation, and add a new field. Same
what I did with AVFormat->filename / AVFormat->url.
Compilation will fail if your code doesn't treat the field as a pointer
after the switch, printing something like
error: 'st->attached_pic' is a pointer; did you mean to use '->'?
123 | st->attached_pic.flags |= AV_PKT_FLAG_KEY;
| ^
So it's not like it will compile and then crash at runtime by accessing
the wrong thing.
After looking a bit more, it will fail if you try to access fields like
i mentioned above, but if you pass it as argument to some function it
will only warn about mismatching types.
So i guess yeah, a new field with a new name would be the cleanest
solution. So as i said, field name suggestions welcome.
But if you still feel more inclined to replace the field, can you
suggest a name for the new one?
_______________________________________________
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".
