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

[Hendrik Leppkes]

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.

- Hendrik