[PVR] Cleanup: Reduce cores/VideoPlayer/Edl.cpp PVR dependencies. #16709
Conversation
ksooo
added
Type: Cleanup
| case Action::CUT: | ||
| case Action::MUTE: | ||
| case Action::COMM_BREAK: | ||
| if (AddCut(cut)) | ||
| { | ||
| CLog::Log(LOGDEBUG, "%s - Added break [%s - %s] found in PVR item for: %s.", | ||
| __FUNCTION__, MillisecondsToTimeString(cut.start).c_str(), | ||
| MillisecondsToTimeString(cut.end).c_str(), CURL::GetRedacted(fileItem.GetDynPath()).c_str()); | ||
| } | ||
| else | ||
| { | ||
| CLog::Log(LOGERROR, "%s - Invalid break [%s - %s] found in PVR item for: %s. Continuing anyway.", | ||
| __FUNCTION__, MillisecondsToTimeString(cut.start).c_str(), | ||
| MillisecondsToTimeString(cut.end).c_str(), CURL::GetRedacted(fileItem.GetDynPath()).c_str()); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Exception: Do not increase indentation after a
switchstatements.
https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#32-indentation
| case Action::SCENE: | ||
| if (!AddSceneMarker(cut.end)) | ||
| { | ||
| CLog::Log(LOGWARNING, "%s - Error adding scene marker for PVR item", __FUNCTION__); | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| CLog::Log(LOGINFO, "%s - Ignoring entry of unknown cut action: %d", __FUNCTION__, static_cast<int>(cut.action)); | ||
| break; |
There was a problem hiding this comment.
Exception: Do not increase indentation after a
switchstatements.
https://github.com/xbmc/xbmc/blob/master/docs/CODE_GUIDELINES.md#32-indentation
xbmc/cores/VideoPlayer/Edl.cpp
Outdated
| @@ -564,8 +564,10 @@ bool CEdl::ReadPvr(const CFileItem &fileItem) | |||
| return !cutlist.empty(); | |||
| } | |||
|
|
|||
| bool CEdl::AddCut(Cut& cut) | |||
| bool CEdl::AddCut(const Cut& NewCut) | |||
There was a problem hiding this comment.
better pass by value instead of making a copy at the begin of the function
There was a problem hiding this comment.
I don't think this would be better. Separate interface from implementation...
There was a problem hiding this comment.
How does this relate to separate interface from implementation?
I'm more with rechi here, passing by value makes it obvious to the caller that it will be copied, without needing to even read into the implementation.
But just stating my opinion, decision is yours ofc.
There was a problem hiding this comment.
I see it more like this: Normal way of doing things is to pass "large" objects by reference. If I pass by value there must be good reason for this. The fact that the current implementation wants to manipulate the parameter's value is 100% implementation detail, has nothing to do with the interface and this does not justify to break the rule to pass by value only in exceptional cases.
Let's assume I change the implementation not to manipulate the parameter's value anymore. I bet, I easily forget to change the parameter to "const &", which I should do for best interface design, because by-value is no longer needed, then. Ooops. Implementation detail controls the interface? Really? Should not be.
There was a problem hiding this comment.
It's like the difference between ModifyAndAddCut(Cut& newCut) and AddCut(const Cut& newCut).
Yes, and this is for another PR. |
…modified value is not used by callers, making it possible to achieve more constness in calling code.
ksooo
force-pushed
the
pvr-refactor-edl
branch
from
f0215ff
to
435c311
Compare
Another small refactoring to reduce compile time references to PVR component.
Runtime-tested on macOS, latest master.