Hi, On Sun, Oct 11, 2015 at 2:35 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Sun, Oct 11, 2015 at 2:33 PM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: > > Hi, > > > > On Sun, Oct 11, 2015 at 2:15 PM, Ganesh Ajjanagadde <gajja...@mit.edu> > > wrote: > > > >> On Fri, Sep 18, 2015 at 5:02 PM, Michael Niedermayer <michae...@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 <gajjanaga...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
