[vlc-devel] [PATCH] http: handle shoutcast/icecast (ICY) metadata

[Ludovic Fauvet]

On Fri, Feb 19, 2016, at 19:06, Rémi Denis-Courmont wrote:
> Le 2016-02-19 13:39, Ludovic Fauvet a écrit :
> > Hello Rémi,
> >
> > On Thu, Feb 18, 2016, at 00:33, Rémi Denis-Courmont wrote:
> >> Le 2016-02-17 18:36, Ludovic Fauvet a écrit :
> >> > This patch targets the new HTTP access module
> >>
> >> The old "HTTP" access is still needed/used for real ICY servers.
> >>
> >> Is there a benefit in using the new HTTP access for ICY? The same 
> >> trick
> >> as for MMSH would avoid having two different implementations of ICY.
> >
> > I wasn't aware of the MMSH trick, it sounds like a very good idea to
> > separate the ICY code in its own module.
> 
> My point was that icyx://... URLs are handled by the old HTTP plugin 
> just fine.
> 
> I don't have any strong opinion on separating ICY from old HTTP.

Following your previous suggestions I'm reimplementing the module as a
stream filter instead. It's most certainly the best choice both for code
clarity and layer separation. Only few lines will be required in the
HTTP code to fetch some headers.

> > It goes without saying this will
> > cause another kind of duplication (4 HTTP implementation instead of 
> > 3)
> > and that it won't work with HTTP/2.0 (do we care?).
> 
> ICY is _not_ HTTP. It's not proper HTTP/1.x and it most certainly won't 
> be HTTP/2.0. If you connect to an ICY server using http:// URI with 
> current vlc.git, the plugin just fails to parse the server response. 
> Open() fails and then the core falls back to the old plugin. In fact, it 
> still works, albeit with 2 extra RTTs.

Nope, it _does_ work with the new module but not in HTTP2 mode of
course.

> > Anyway, my first thought was that we were going to phase-out the 
> > legacy
> > HTTP module in the upcoming months (and kill it with fire) and that
> > having both implementation (a very intrusive one in legacy and a
> > less-intrusive one in the new module) wouldn't really be a problem.
> 
> > I
> > don't know what's your timeline on this matter, maybe you have other
> > plans for the old HTTP module.
> 
> My plan is to remove old non-ICY bits from the old HTTP module as soon 
> as the missing HTTP features are implemented:
> - proxying of non-TLS origins,
> - basic authentication.
> 
> If someone splits ICY out of old HTTP first, then I'll just remove old 
> HTTP completely.

Then you'll be able to remove the old HTTP altogether.

> > The ideal solution would probably involve the usage of primitives 
> > from
> > the new HTTP module within other access modules but it's probably too
> > much to ask for VLC 3.0 and I'm having a hard time figuring out how 
> > much
> > work would be required to do so.
> 
> FAICT, ICY protocol syntax is not compatible with HTTP, so that won't 
> work.
> 
> > The problem is, currently ICY streams do not work anymore because the
> > new HTTP has precedence over the legacy one.
> 
> They do? I get "HTTP connection failure" and then the old plugin plays 
> the stream as before.

The stream given in this bug report goes through the new module, not the
old one.
https://code.videolan.org/videolan/vlc-android/issues/28

-- 
Ludovic Fauvet
www.videolan.org