use openrtb2.video instead of request.video in native asset

[Wwwmmxxx]

the annotation in /native1/request/video.go writes "For optional attributes please refer to OpenRTB.", i suppose the fields that list in native-specification is not enough, and it is better to use openrtb2/video.go instead of /native1/request/video.go in Assets struct.
am i right? Or the extra feilds should be added into video.ext ?

[mxmCherry]

Looks to be another example of imperfect OpenRTB/Native spec, congratulations for bumping into this 🎉

I'm looking into docs / options, but unfortunately I don't think we can do much.

[mxmCherry]

So, in latest master, we already have:~~

• openrtb2.Video: detailed video object)
• adcom1.Video: that's for openrtb3; it's similar, but not strictly equal to openrtb2.Video - the openrtb2 thing has "mimes" vs adcom1 has "mime" and maybe more, did not check others
• native/request.Video: looks to be just a required-only-field version openrtb2.Video (looking into Native 1.2 and it has "mimes" field, so it's not modelled after adcom1 - not available at the time of Native 1.2, IIRC).

I can suggest to simply copy attributes of openrtb2.Video -> native/request/openrtb2.Video and consider it done:

1. I don't want native1 to depend on openrtb2 (it's actually the opposite)
2. Given that IAB specs are usually quite messy, copy-paste is the (only) normal way to deal with it

Feel free to PR.

Nope, this would drag half of openrtb2 into native/request.

[mxmCherry]

~~Update: if you (or anyone else) is going to PR this - pls branch from / PR against https://github.com/mxmCherry/openrtb/tree/v15 ~~

I will not consider adding attributes a major version change, so better have it in stable v15 so lib users can benefit from it ASAP (without having to handle more changes in v16).

Nope, see above - it'll drag half of openrtb2 into native/request.

[mxmCherry]

Now we have even more problems.

If Native is used with OpenRTB 2.x - fine, it can embed openrtb2.Video (I'm trying to consider this option).

But if Native is used with OpenRTB 3.0 / AdCOM 1.0, should it use the openrtb2.Video?

Or maybe some exchanges will not follow the doc strictly and will use adcom1.Video for native1/request.Asset.Video when Native is used with OpenRTB 3.0? (that would be very natural, and I remember some exchanges deviating from weird decision/mistakes in IAB spec).

So maybe the only way to overcome all this is to switch native1/request.Asset.Video to json.RawMessage so payload can be parsed into any of the 3 mentioned types (using generics in this lib would be super-complex as there are many use-cases for them + not everyone uses G0 1.18 already).

And this definitely would be a major version bump, so would be delivered a bit later (with plenty of OpenRTB 2.6-related changes).

@Wwwmmxxx any ideas? I think json.RawMessage is the best way for it, maybe missing something simile 🤷

[soloctl]

Seem open rtb 2.x and 3 not designed to sit in the same branch or repo to me?

[mxmCherry]

No, they are fine to be in one repo - that doesn't matter. What's not very clear here is the Native spec. It would be easier if it wouldn't point to OpenRTB spec (because - which version?), but copy types into Native spec itself.

So, theoretically, Native's Video can be either OpenRTB 2.x or OpenRTB 3.x. Depends on exchange, I guess - there's too much "freedom" in Native spec (at least in this exact case).

And to accomodate this, using json.RawMessage seems to be the only proper solution, so lib user can parse Native response either into openrtb2.Video or into adcom1.Video (AdCOM is basically extracted/split out from OpenRTB 3 and it's not 100% compatible with OpenRTB 2 - field renames etc).

[Wwwmmxxx]

Thanks for the Reply !

currently, i have no idea about how to deal with the problem. it seems like, using json.RawMessage is the only way, otherwise, we will drag openrtb2 in native/request

Another question, Is it possible that openrtb3.0 and adcom1 does not use native 1.2 specification. Lately i read them in github respository ( https://github.com/InteractiveAdvertisingBureau/openrtb, https://github.com/InteractiveAdvertisingBureau/AdCOM ) since i know nothing about them before.

the native object in AdCOM notes that This object is the root of a structure that defines a native display ad. , not referring to native 1.2 specification, what's more, i cannot find keywords (native specification) in the article. Then, i campared openrtb2.5/native with adCOM/native, i suppose the native 1.2 specifation is partly mixed into adcom.

If i said somehing wrong, please correct my mistake

[mxmCherry]

Valid point 🤔

Maybe I've tricked myself regarding this one, haven't used OpenRTB 3.

Then, importing openrtb2 in native1/request for video message seems suitable, but I'll better leave it till weekend to have some time to think about it to be 100% sure. I'm still not happy about native1 depending on openrtb2, though it seems logical in this case, as adcom1 has it's own Native/Video objects.

[Wwwmmxxx]

Im on your side. each specification should be indenpendent from others, so that it can be used more widely.

maybe, emmmm, updating Native/Video struct the same as Openrtb2/Video is a great solution

[mxmCherry]

@Wwwmmxxx 'openrtb2.Video` embeds quite a bunch of other types (both enums and objects), so it would be too much to copy.

I think introducing such a dep is OK-ish: luckily, openrtb2 doesn't embed Native right now: https://github.com/mxmCherry/openrtb/blob/v13.0.0/openrtb2/native.go#L21-L27 (it comes in as JSON string to be parsed separately - that's a weird decision, but anyway).

For now, I'm open to dropping native1/request.Video type and importing/using openrtb2.Video instead. Just add some // Note: ... on the affected field. Or just do a type alias (as a documentation feature in this case), it's supported in Go 1.9 - doubt anyone uses such an ancient version anywhere (https://go.dev/doc/go1.9#language), either approach is fine.

I'm convinced by #52 (comment)

So let's just YOLO it - switching to json.RawMessage can be done any time later if anyone has issues with it (I doubt it though).

Feel free to PR.

[mxmCherry]

Released the change as https://github.com/mxmCherry/openrtb/releases/tag/v17.0.0-alpha.1 (final v17 to be tagged in a few days).

[mxmCherry]

Released as final v17: https://github.com/mxmCherry/openrtb/releases/tag/v17.0.0