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

[Jan Ekström]

On Thu, Jan 14, 2021 at 1:40 PM Nicolas George <george at nsup.org> wrote:
>
> Jan Ekström (12021-01-13):
> > Yea, I actually noticed that one, but never got to responding to it.
> > Sorry about that.
>
> No problem.
>
> > While I do agree that such modes could be useful, currently I have no
> > need for the additional modes and thus the original code by Stefano
> > was just moved in avutil (and then utilized from ttmlenc). I did check
> > the XML specification that this code originally seemed to follow the
> > requirements (and thus mentioned the exact spot in the spec where this
> > is mentioned).
>
> Yes, it is technically correct. So would be escaping every character as
> a numeric entity.
>
> &#x41;&#x64;&#x74;&#x61;&#x27;&#x20;&#x6f;&#x20;&#x65;&#x6c;&#x79;&#x72;&#x61;&#x61;&#x6c;&#x2c;&#x69;&#x20;&#x74;
>
> XML is meant to be both machine-readable and human-readable. Escaping
> unnecessarily pulls us away from human-readable.

I did not do the selection of what this code (which I just moved)
does, I just moved it and referred to the originator document that
those five symbols match what is defined in the XML spec as per 2.4.

As far as I can tell, it does not do nor was I implying that I would
do something like that.

>
> > Thus, is the lack of those additional XML modes a blocker? Or should
> > they be added as the need arises?
>
> Considering that this is so simple that it would have taken you less
> time to just do it than to wait for my answer, I would say that I insist
> to do things properly immediately, before code starts piling on it and
> requires to be revised.
>

Alright. I only took this position since it required to check out the
XML specification again to see if that stuff was properly defined (it
probably is?), so there wouldn't be guesswork required and rather just
implementing the rules. I can check the spec again, and see if it
clearly defines those cases that you have mentioned.

Jan