On Mon, Jan 25, 2021 at 2:47 PM Nicolas George <geo...@nsup.org> wrote: > > Jan Ekström (12021-01-25): > > For now I kept them separate since one is just moving Stefano's code, > > and another adds new functionality that you requested. > > Stefano's code was good enough for ffprobe, because it had to perform a > very precise task in a very constrained context, but as is, it is not > good enough for lavu, it needs to be adapted. >
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. But at this point I have wasted enough time with parts of this patch set that I do not care about. I do wish that I had said to you earlier that if you want to rework this stuff, you could do it yourself. I just noticed the existing code and attempted to do the Right Thing and move it into general code. I bet if I had not done it, nobody would have bat an eye regarding it, since that's exactly how the existing XML output things do things right now. I hope it is understandable why it is annoying to me that instead of the code that I actually proposed to add, the stuff that was *already* *there*s is the stuff that gets now picked out. > > I really have no bigger bone with either naming, I just utilized: > > 1. How the XML spec calls the thing ("AttValue" thus becoming > > ATT_VALUE, "CharData" becoming CHAR_DATA). I slightly would prefer to > > keep this due to this making it easy to point towards what is actually > > meant in the XML spec. > > CharData is only used a few times in the abstract syntax of the spec, it > will have meaning only for people who are very familiar with it. > It also contains a nice definition of it in 2.4 "[Definition: All text that is not markup constitutes the character data of the document.] " > > 2. Single and double quotes are the wording that I am more acquainted > > with, but I have no hard opinion so if you want _QUOT/_APOS, sure. > > I am fine with "double_quotes" and "single_quotes" or "dquot" and > "squot" too. > This is where (for now) I go through the route of least resistance and post a v5 https://github.com/jeeb/ffmpeg/commits/ttml_encoder_v5 - I changed "quoted" to "quotes". - 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". - I removed the double and single quote escaping from the CHAR_DATA escaping, as you requested. 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. Best regards, Jan _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".