[FFmpeg-devel] [PATCH 3/9] lavf/ffm: store/restore private codec context

[Michael Niedermayer]

On Fri, Nov 14, 2014 at 11:33:08PM +0100, Lukasz Marek wrote:
> On 12.11.2014 02:12, Michael Niedermayer wrote:
> >On Tue, Nov 11, 2014 at 11:59:32PM +0100, Lukasz Marek wrote:
> >>On 11.11.2014 14:46, Michael Niedermayer wrote:
> >>>the muxer might run in a seperate thread from the encoder, priv_data
> >>>could change while its being accessed, iam not sure if that would
> >>>cause some race here or not
> >>
> >>Wouldn't it regard AVCodecContext.variables too?
> >
> >potentially yes for fields which are not expected to be accessed
> >
> >
> >>
> >>>Also i dont think its guranteed that priv_data starts with a AVClass
> >>
> >>Fixed patch attached
> >>
> >
> >>  avformat.h |    7 +++++++
> >>  ffmdec.c   |   36 +++++++++++++++++++++++++++++++-----
> >>  ffmenc.c   |   34 ++++++++++++++++++++++++++++++++--
> >>  utils.c    |    1 +
> >>  4 files changed, 71 insertions(+), 7 deletions(-)
> >>3807f44019d60c346dee78e968e195ab783565ee  0001-lavf-ffm-store-restore-private-codec-context.patch
> >> From af7c72d8d62e26c4b8fe9d8a758707532e727830 Mon Sep 17 00:00:00 2001
> >>From: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> >>Date: Mon, 10 Nov 2014 23:22:59 +0100
> >>Subject: [PATCH 1/2] lavf/ffm: store/restore private codec context
> >>
> >>TODO: bump minor
> >>
> >>Signed-off-by: Lukasz Marek <lukasz.m.luki2 at gmail.com>
> >>---
> >>  libavformat/avformat.h |  7 +++++++
> >>  libavformat/ffmdec.c   | 36 +++++++++++++++++++++++++++++++-----
> >>  libavformat/ffmenc.c   | 34 ++++++++++++++++++++++++++++++++--
> >>  libavformat/utils.c    |  1 +
> >>  4 files changed, 71 insertions(+), 7 deletions(-)
> >>
> >>diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>index 3733549..474192d 100644
> >>--- a/libavformat/avformat.h
> >>+++ b/libavformat/avformat.h
> >>@@ -905,6 +905,13 @@ typedef struct AVStream {
> >>      int event_flags;
> >>  #define AVSTREAM_EVENT_FLAG_METADATA_UPDATED 0x0001 ///< The call resulted in updated metadata.
> >>
> >>+    /**
> >>+     * String containing paris of key and values describing recommended encoder configuration.
> >>+     * Paris are separated by ','.
> >>+     * Keys are separated from values by '='.
> >>+     */
> >>+    char *recommended_encoder_configuration;
> >
> >this requires accessor functions for future ABI compatibility
> >also it should be documented what and who sets it, unless its
> >intended to be changed how its set then it could be documented later
> >when the final design is implemented
> 
> I added these accessors.
> Note: I added this field in public API section. From this comment I
> can suspect I should be private, so please verify.

yes and no
it should be moved to the end but thats because of bugs, i think
there are still some applications that access private fields and
we should avoid moving these
otherwise it could be argued to be on either side (public with a
note that access is only through the accessors or on the private
side)

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141115/e764f2ef/attachment.asc>