[FFmpeg-devel] [PATCH][RFC] ffmpeg: remove access to private FILE struct members on Windows

[Hendrik Leppkes]

On Tue, Aug 4, 2015 at 7:33 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Tue, Aug 04, 2015 at 01:04:34PM -0400, Ronald S. Bultje wrote:
>> Hi,
>>
>> On Tue, Aug 4, 2015 at 12:42 PM, Hendrik Leppkes <h.leppkes at gmail.com>
>> wrote:
>>
>> > On Mon, Aug 3, 2015 at 9:54 PM, Hendrik Leppkes <h.leppkes at gmail.com>
>> > wrote:
>> > > On Mon, Aug 3, 2015 at 9:30 PM, Nicolas George <george at nsup.org> wrote:
>> > >> Le sextidi 16 thermidor, an CCXXIII, Michael Niedermayer a écrit :
>> > >>> i also found this comment in a patch in my inbox:
>> > >>>
>> > >>> +  /* When using Standard C input functions, also check if there
>> > >>> +   is anything in the buffer. After a call to such functions,
>> > >>> +   the input waiting in the pipe will be copied to the buffer,
>> > >>> +   and the call to PeekNamedPipe can indicate no input available.
>> > >>> +   Setting stdin to unbuffered was not enough, IIRC */
>> > >>> +  if (stdin->_cnt > 0)
>> > >>> +  {
>> > >>> +    char ch;
>> > >>> +    //Read it
>> > >>> +    read(0, &ch, 1);
>> > >>> +    return ch;
>> > >>> +  }
>> > >>>
>> > >>> This seems to explain what the code was intended to do
>> > >>
>> > >> That was my guess.
>> > >>
>> > >> This is ugly, and should be removed anyways. It is not possible to mix
>> > stdio
>> > >> and low-level reading like that, period.
>> > >>
>> > >> Fortunately, ffmpeg.c only uses stdin at one single place, a few lines
>> > below
>> > >> the corresponding code:
>> > >>
>> > >>         }else
>> > >>             if(scanf("%d", &debug)!=1)
>> > >>                 fprintf(stderr,"error parsing debug value\n");
>> > >>
>> > >> It needs to be changed to read a line with the low-level function
>> > >> (read_key()), exactly like 40 above for the 'c' case. I can not brew a
>> > patch
>> > >> right now, sorry.
>> > >>
>> > >> I suspect this would not work:
>> > >>
>> > >> printf 'd42\nC ... fontfile ...\n' | ffmpeg ...
>> > >>
>> > >> because the C will end up in the stdio buffer.
>> > >>
>> > >>
>> > >
>> > > Good catch. This command doesn't work with or without the hunk this
>> > > patch removes however.
>> > > A simpler command without drawtext would be:
>> > >
>> > > printf 'd0\n?' | ffmpeg -i ...
>> > >
>> > > If you see help output, it works. Using another command instead of d0
>> > > works fine, like a C command followed by a \n?
>> > > It also doesn't work on Linux, so thats a more fundamental problem.
>> > >
>> > > Since this fixes a build failure, unless we can find a use-case where
>> > > this hunk actually does make a difference, I would still rather remove
>> > > it.
>> > > If it causes any regression in the future, which we didn't think of
>> > > now, I'll be here to work on a fix.
>> > >
>> >
>> > Any further thoughts on this?
>> >
>> > I would love start setting up a VS2015 FATE box etc, once compilation
>> > actually works.
>>
>>
>> I thought the conclusion was to apply it? I certainly support that.
>
> +1
>

Applied.