[FFmpeg-devel] [PATCH v4 1/4] avutil/{avstring, bprint}: add XML escaping from ffprobe to avutil

[Nicolas George]

Jan Ekström (12021-02-01):
> I beg to differ, since it seemed to fully implement the cases noted in
> 2.4. Does escape double and single quotes unnecessarily in some cases,
> yes. But not invalid.

Not invalid, but not good enough.

> https://github.com/jeeb/ffmpeg/commits/ttml_encoder_v5
> 
> - I changed "quoted" to "quotes".

I strongly suggest you merge the commits, but I will not block on it.

> - I kept the names matching to the spec, esp. due to the definitions
> in the spec in 2.3 and 2.4, as well as kept the fact that it is
> escaping for the actual value of the attribute, not just "the
> attribute".

"ATT_VALUE" is a terrible part of the name, virtually impossible to
memorize. Nobody learns the names used by the XML spec by rote. Call
these modes:

AV_ESCAPE_MODE_XML
AV_ESCAPE_MODE_XML_DOUBLE_QUOTES
AV_ESCAPE_MODE_XML_SINGLE_QUOTES

and be done with it.

This is a public API, it needs to be good immediately.

> - I removed the double and single quote escaping from the CHAR_DATA
> escaping, as you requested.

Thank you very much.

> For the record, I have no great interest in XML. If you wish to take
> this stuff further, please do so. But at this point I am feeling
> rather stumped that after the initial reviews which actually were
> concerned of code that I wrote, most of the comments have been
> regarding things that were already there in the code base.

As for me, I am quite stumped that you wasted your time arguing
something you have no great interest in, instead of just following
advice and be done with it.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210201/c9225167/attachment.sig>