[FFmpeg-devel] [PATCH] avformat/rtmpcrypt: fix discarded const warning

[Ronald S. Bultje]

Hi,

On Sun, Oct 11, 2015 at 2:35 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
wrote:

> On Sun, Oct 11, 2015 at 2:33 PM, Ronald S. Bultje <rsbultje at gmail.com>
> wrote:
> > Hi,
> >
> > On Sun, Oct 11, 2015 at 2:15 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
> > wrote:
> >
> >> On Fri, Sep 18, 2015 at 5:02 PM, Michael Niedermayer <michaelni at gmx.at>
> >> wrote:
> >> > On Fri, Sep 18, 2015 at 09:50:29PM +0200, Nicolas George wrote:
> >> >> Le jour du Génie, an CCXXIII, Ganesh Ajjanagadde a écrit :
> >> >> > This patch silences a -Wdiscarded-qualifiers observed with GCC 5.2.
> >> >> >
> >> >> > Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> >> >> > ---
> >> >> >  libavformat/rtmpcrypt.c | 2 +-
> >> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> I am not sure this is correct: the buffer is const for a reason, the
> >> warning
> >> >> is right. An application would be completely allowed to give a
> buffer in
> >> >> read-only memory, or to reuse the contents of the buffer afterwards.
> >> >>
> >> >> Actually, I suspect this muxer, if used as first output in the tee
> >> muxer,
> >> >> would cause the next outputs to be corrupt.
> >> >
> >> > IIRC the code is safe, just ugly
> >> > the writing only occurs if handshaked is set, which is only set
> >> > by ff_rtmpe_update_keystream() which is not part of the public
> >> > interface and only called from libavformat/rtmpproto.c
> >> > i assume but did not double check that libavformat/rtmpproto.c
> >> > calls the functions so that writable buffers are used
> >> >
> >> >
> >> >>
> >> >> The correct fix would probably be to allocate a new buffer, probably
> >> keeping
> >> >> it in the context for performances reasons instead of allocating each
> >> time.
> >> >
> >> > id need to double check but i think the calling code possibly uses
> >> > the written buffer with the expectation that it has been updated
> >> >
> >> > if that is so then such fix would break it.
> >>
> >> Have you checked the code and confirmed that this patch is fine?
> >
> >
> > I don't think the explanation makes the patch fine, the explanation just
> > says that there's no actual issue being hidden behind the warning. It
> > sounds like this code needs some refactoring...
>
> Unfortunately, I can't see a clean solution to this without changing some
> API.


Right, sorry, I was unclear; I do indeed think we need some API changes to
allow calling this code with a const buffer, with the actual constraint
being that it indeed does not get updated (which, according to Michael, is
usually the case), and then an additional second call which has a non-const
buffer that gets updated (as expected by rtmp).

Ronald