[FFmpeg-devel] [PATCH 6/8] lavf/http: increase range for listen, handle connection closing accordingly, add http_accept, add http_handshake and move handshake logic there

[Nicolas George]

Le tridi 3 thermidor, an CCXXIII, Stephan Holljes a écrit :
> I wasn't sure if in the future something might want to filter the
> different server modes so using 3 would mess up bitmasking. By
> introducing a new field as you suggested, this would become obsolete
> anyway.

This is internal code, and therefore not subject to compatibility
considerations: if someone needs it to be a bitmask, they can just change
the value, it takes about two seconds.

> This would probably make more sense. I am somewhat hesitant to just
> add more and more fields to the HTTPContext; am I worrying too much
> about memory footprint here?

In this instance, yes. If we worried about a few ints in contexts, then we
would start ordering fields in structures to avoid padding, then use smaller
types and bitfields, then allocate private contexts with public ones, etc.

> I merely reused the already present field http_code, should this be avoided?

Depends on what makes the code simpler.

In this kind of cases, I would say: if in doubt, do not reuse a field.
Consider this: field x is used in client code; you reuse it in server code
and test extensively; later, someone extents the client code with extra uses
of field x, except the code they change is also used by the server:
conflict.

> Ok. Should this style be introduced when possible? Then I will make
> the switch-style in http.c consistent once this patchset is done.

If you want, especially if it is code you recently added (sorry for not
spotting it earlier), but do not spend much time on cosmetics.

> _C_lient _L_ower protocol. Is there something better that you can suggest?

Ok.

> While we're at it: What does the "hd" in HTTPContext abbreviate? I
> can't think of anything but http descriptor?

Sorry, no idea. This was added before my time.

> Alright, I didn't think about this enough, probably because I am very
> unfamiliar with how handshakes in the lower protocols (especially TLS)
> work, I will read up on that.

Only if you are interested in it: for the task ahead, the only thing that is
_necessary_ to know is that any protocol handshake can contain one or
several points where the application may need to take control to make a
decision, i.e. several steps where avio_handshake() must return entirely.

> This is a great suggestion, I had trouble deciding where to use HTTP
> status codes and where to use AVERROR HTTP errors.

This question is a bit confused, yes.

> If cc->listen is not set, process_line() will act like a client and
> send client responses to a client. That does not make sense.
> The usage of HTTP_MULTI_CLIENT can be substituted by anything but 0
> (after re-reading the code, I am fairly certain even 1 can be used),
> and as it was mentioned before this is better handled by a separate
> field and thus also becomes obsolete.

Ok.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150722/906a90a9/attachment.sig>