enum InterPredMode and enum CompInterPredMode: make real enums
#927
Conversation
| impl From<InterPredMode> for CompInterPredMode { | ||
| fn from(value: InterPredMode) -> Self { | ||
| match value { | ||
| InterPredMode::Nearest => CompInterPredMode::NearestNearest, | ||
| InterPredMode::Near => CompInterPredMode::NearNear, | ||
| InterPredMode::Global => CompInterPredMode::NearestNew, | ||
| InterPredMode::New => CompInterPredMode::NewNearest, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this conversion happens in practice, though I don't really understand why it makes sense. Maybe the CompInterPredMode field is just implicitly a InterPredMode in certain circumstances? or there is something more subtle going on.
There was a problem hiding this comment.
but e.g. Global mapping to NearestNew is weird when GlobalGlobal exists.
There was a problem hiding this comment.
That is very weird. I have no idea why that's done. It looks like a correct translation of the C, though. @fbossen, have any idea?
Maybe the same fields are being used for the two enums, but never at the same time? I'm curious if the tests would still pass if we changed the conversion to what would be expected.
There was a problem hiding this comment.
there is this one expression that mixes the two types
(b.inter_mode() == if is_comp { GLOBALMV_GLOBALMV } else { GLOBALMV }) as c_int;so, I guess, if is_comp is true then CompInterPredMode is used, and otherwise just InterPredMode. That at least intuitively makes sense here, and also seems to be true in this file.
Not sure what to do with that though, is_comp is a runtime value so it's hard encode that into the type. Some enum { Comp(_), NotComp(_) } enum also doesn't seem great (but would provide some extra type safety).
| Nearest = 0, | ||
| Near = 1, | ||
| Global = 2, | ||
| New = 3, |
There was a problem hiding this comment.
I've kept the number here for now because the ordering is important for the mapping to CompInterPredMode
| impl From<InterPredMode> for CompInterPredMode { | ||
| fn from(value: InterPredMode) -> Self { | ||
| match value { | ||
| InterPredMode::Nearest => CompInterPredMode::NearestNearest, | ||
| InterPredMode::Near => CompInterPredMode::NearNear, | ||
| InterPredMode::Global => CompInterPredMode::NearestNew, | ||
| InterPredMode::New => CompInterPredMode::NewNearest, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
That is very weird. I have no idea why that's done. It looks like a correct translation of the C, though. @fbossen, have any idea?
Maybe the same fields are being used for the two enums, but never at the same time? I'm curious if the tests would still pass if we changed the conversion to what would be expected.
folkertdev
force-pushed
the
folkertdev/drl-proximity-enum
branch
from
32f97e8 to
f10d302
Compare
folkertdev
force-pushed
the
folkertdev/inter-pred-mode-enum
branch
from
5711bfe to
9743090
Compare
No description provided.